* cleanup: remove unused latestSnapshotSeq from HTTP response and client types
latestSnapshotSeq was computed on every download but never read by any
client code. sibling snapshotVectorClock IS consumed — only this field
is dead. The server still computes it internally for the snapshot
optimization logic, but it no longer serializes it in the response.
Removed from:
- DownloadOpsResponse type and response object (sync.routes.ts)
- Zod schema (supersync-http-contract.ts) + associated test
- OpDownloadResponseBase interface (sync-providers)
- Client validator test + stale comment references
- Server integration test assertions
* cleanup: remove deprecated CONFLICT_STALE error code
Server never emits CONFLICT_STALE (fully migrated to
CONFLICT_SUPERSEDED).
* cleanup: remove unused SyncConfig fields (maxOpsPerUpload, downloadLimit, downloadRateLimit)
These three config fields were declared and defaulted but never read by
any code. Routes hardcode their own limits:
- maxOpsPerUpload: gate is MAX_OPS_PER_BATCH constant
(sync.routes.payload.ts)
- downloadLimit: hardcoded Math.min(limit, 1000) in sync.routes.ts
- downloadRateLimit: hardcoded max:200 in route rateLimit config
* cleanup: remove dead/drifted duplicate types from sync.types.ts
Removed 6 types with zero references repo-wide:
- SnapshotResponse: orphaned after previous PR removed its importers
- UploadSnapshotRequest: zero uses, missing 7 fields vs actual Zod
schema
- RestorePointType: zero uses, included dead 'DAILY_BOUNDARY' value
- RestorePoint: zero uses (live version in snapshot.service.ts)
- RestorePointsResponse: zero uses
- RestoreSnapshotResponse: zero uses
Retire GET /api/sync/snapshot (attack-surface reduction):
- Remove route handler from sync.routes.ts (no production client caller)
- Keep internal generateSnapshot() and all downstream code
- Update tests: delete GET /snapshot describe block, rework isolation
test (sync.routes.spec.ts), remove GET assertion (sync-fixes.spec.ts),
remove SimulatedClient.getSnapshot() helper + rework 2 integration tests
to use GET /api/sync/ops instead (multi-client-sync.integration.spec.ts)
Re-scope GET /api/sync/status as diagnostic:
- Add doc comment to route handler marking it diagnostic
- Update all documentation (README, API wiki, architecture diagrams)
Documentation updates across 5 files remove GET /snapshot references
and label GET /status as diagnostic.
Breaking: self-hosters with external tooling relying on GET /snapshot
must migrate — no shipped client version ever called this endpoint.
The entity-ids-conflict PGlite spec hand-copied the conflict-detection SQL
but had drifted to the old mutually-exclusive
`CASE WHEN cardinality(entity_ids) > 0 THEN entity_ids ELSE ARRAY[entity_id]`
form, while production conflict.ts now uses the
`entity_ids || ...ARRAY[entity_id]` UNION (the silent-data-loss fix in
560757b88f). The drifted spec passed with either form, so it gave false
assurance, and the only test that actually discriminated the fix lived in a
DATABASE_URL-gated, vitest-excluded integration spec that runs in no CI job.
Sync both unnest sites to the production union SQL and add a divergent-scalar
regression to detectConflictForEntities and prefetchLatestEntityOpsForBatch:
an op whose scalar entity_id (task-Z) is NOT a member of its own entity_ids
must still be found. Verified the new cases fail on the old CASE SQL and pass
on the union (12/12 green).
The two raw-SQL latest-writer lookups unnested a mutually-exclusive
`CASE WHEN cardinality(entity_ids) > 0 THEN entity_ids ELSE ARRAY[entity_id]`,
so a stored op whose scalar entity_id is NOT a member of its own entity_ids
(producible via getStoredEntityIds dedup) was invisible to the batch lookup -
a later concurrent op touching that entity was wrongly accepted (silent data
loss). This only bites once batch upload is enabled in prod, but is a real
latent divergence between the single-entity (Prisma OR) and batch (raw SQL)
paths.
Fold the scalar into the set with a union:
entity_ids || CASE WHEN entity_id IS NULL THEN '{}'::text[] ELSE ARRAY[entity_id] END
(entity_id is nullable; the guard prevents array||NULL nulling the whole
array). DISTINCT ON dedupes the common entity_id = entityIds[0] overlap. The
WHERE prefilter is unchanged, so GIN(entity_ids) + entity_id btree usage is
preserved. Update the conflict-detection.spec mock to union too, and add a
DATABASE_URL-gated PG integration test exercising the real detectConflict /
prefetchLatestEntityOpsForBatch (divergent-scalar case fails under the old
CASE); excluded from the default vitest run like the sibling SQL integration test.
* test(sync): cover multi-entity conflict SQL against real Postgres (#8334)
Add an in-process Postgres (PGlite) spec that runs the actual unnest(CASE...)
conflict-detection SQL from conflict.ts, covering the non-first-entity case,
latest-per-entity selection, scalar fallback for pre-migration rows, and the
NULL-entity_id full-state op. Unlike the DATABASE_URL-gated integration specs
(excluded from the default config), this runs in the normal npm test job.
* fix(sync): delete all entityIds on multi-entity DEL replay (#8340)
Server op-replay deleted only the scalar entityId, so a batch delete
(deleteTasks stores its full set in entityIds, scalar = entityIds[0]) left
entities 2..n alive in restore snapshots, which feed back to clients. Thread
entityIds (now persisted, #8334) through replay and delete the full set, with
the same prototype-pollution guard and scalar fallback for single-entity and
pre-migration ops. Add entityIds to REPLAY_OPERATION_SELECT.
* fix(sync): conflict-check all entities of multi-entity ops #8334
Multi-entity ops (deleteTasks, moveToArchive, __updateMultipleTaskSimple,
round-time-spent, batch board/issue-provider actions) carry entityIds[], but
the server operations table only persisted the scalar entity_id (= entityIds[0]).
Once such an op was stored, only its first entity took part in future conflict
lookups, so a later stale write to a non-first entity found no prior writer and
was wrongly accepted instead of rejected as CONFLICT_SUPERSEDED/CONCURRENT.
- Add an entity_ids text[] column (populated for multi-entity ops only via
getStoredEntityIds; single-entity ops store [] and use the scalar) + a GIN
index. Migration is metadata-only with no backfill: pre-migration rows fall
back to entity_id, so the fix is forward-only (entities 2..n of already-stored
ops were never persisted and stay unrecoverable).
- detectConflictForEntity now runs two ordered LIMIT-1 lookups (scalar btree +
entity_ids GIN) and takes the higher server_seq, preserving the fast ordered
hot path instead of an OR's BitmapOr+sort.
- detectConflictForEntities / prefetchLatestEntityOpsForBatch match an entity as
the scalar entity_id OR a member of entity_ids (unnest CASE + && / = ANY).
- Harden validateOp to bound entityIds (length + per-element), mirroring entityId.
Raw SQL validated against Postgres (PGlite) incl. GIN usage and two-query
correctness; single path + validation + migrations covered by unit tests. The
full conflict-detection.spec needs a generated Prisma client (CI), and the
hot-path round-trip tradeoff should be confirmed with a real-PG EXPLAIN.
* fix(sync): store entity_ids when a batch op dedups off the scalar #8334
Multi-review follow-up. getStoredEntityIds gated on `length > 1`, so a batch op
whose entityIds dedup to a single value that differs from entityId (the server
does not enforce entity_id === entityIds[0]) stored [] and that entity became
invisible to conflict lookups — reintroducing #8334 for it. Gate on "is the set
exactly [entity_id]?" instead, and cover it with unit tests.
Also: correct the array-branch comment/doc — a GIN(entity_ids) lookup has no
server_seq so it match-all-then-sorts (cheap only because multi-entity ops are
rare), it is not an ordered walk; drop a stale "OR filter" test docstring; add a
counter-note on getConflictEntityIds vs getStoredEntityIds to prevent swapping.
* refactor(sync): single OR lookup for entity conflict detection #8334
Third multi-review follow-up. Revert detectConflictForEntity from the two ordered
findFirst lookups back to a single Prisma OR [{entityId}, {entityIds:{has}}]. The
two-query split optimized the OR's BitmapOr+sort, but that is bounded by op-log
pruning (sub-ms in practice) while the split added a guaranteed extra round-trip on
the common single-entity path (doubled by the FIX-1.5 re-check) — a net-negative for
the median. The OR is simpler, fully typed/testable, and likely faster in aggregate;
a code comment documents the split as the escalation if a real-PG EXPLAIN ever shows
the OR is a problem.
Review polish (no behaviour change): correct the array-branch/getStoredEntityIds
comments (a GIN @> is match-all-then-sort, not an ordered walk; multi-entity-only
storage's win is GIN size + keeping single-entity inserts off the GIN, not sort
depth); note the batch unnest paths as the first EXPLAIN candidates under load; keep
the two batch queries' CASE/prefilter SQL inline (a shared fragment would shift the
positional params the conflict-detection.spec mock relies on) with a keep-in-sync
note; replace a non-ASCII <= in a client-facing validation error string.
* test(sync): update db mocks for OR + prefetch entity_ids lookups #8334
Running the full super-sync-server suite (with a generated Prisma client) surfaced
two specs whose inline db mocks hadn't tracked the new conflict-detection SQL:
- time-tracking-operations.spec: findFirst now models the single-entity lookup's
OR: [{entityId}, {entityIds:{has}}] shape (it previously only matched the scalar
entityId, so a concurrent single-entity update was wrongly "accepted").
- sync.service.spec: the prefetch $queryRaw mock parsed userId as the last param and
flattened all params into the touched pairs; the #8334 prefilter adds idArray
params after userId, so it now finds userId by type and reads the touched pairs
from the VALUES join fragment only.
Production code unchanged; these are test-mock fidelity fixes. Full suite: 800 passing.
validateOp enforced the per-payload size limit with JSON.stringify().length
(UTF-16 code units) while the quota gate, the persisted payloadBytes column,
and the storage counter all measure UTF-8 bytes (Buffer.byteLength). Non-ASCII
payloads undercount in the check, so a payload could pass validation yet be
charged more everywhere else.
Switch the check to Buffer.byteLength(...,'utf8') and reuse the measured size
so a large payload is stringified once for sizing instead of repeatedly per
upload:
- computeOpStorageBytes gains an optional cachedPayloadBytes arg.
- validateOp returns payloadBytes; both upload paths reuse it at the persist
site (the vector clock is still measured there since it is pruned after
validation, so the stored clock differs from the validation/gate-time clock).
- processOperation returns { result, storageBytes, fallback } so the serial
loop reuses the size instead of recomputing it, mirroring the batch path's
acceptedDeltaBytes.
Net payload stringifies per accepted op: serial 4->2, batch 3->2 (the
pre-validation quota gate remains the one unavoidable measure).
Behavior change: heavily non-ASCII full-state imports above 20MB UTF-8
(previously accepted under the looser UTF-16 count) are now correctly rejected
with PAYLOAD_TOO_LARGE, matching the documented byte limit.
A rolled-back upload (INTERNAL_ERROR) was cached under the deterministic
requestId, so retries of the same batch were served the cached failure for
the 5-min dedup TTL — defeating retries. Skip caching INTERNAL_ERROR results
in both the ops and snapshot handlers. #8332
The Prisma migration chain began with an ALTER on the operations table, so
a fresh database could not be initialized from migrations alone (the first
migration failed with no table to ALTER). Separately, the login_token /
login_token_expires_at columns and index existed in schema.prisma and were
used at runtime by src/auth.ts (magic-link login) but were never created by
any migration, so a migrate-only database crashed with
"column users.login_token does not exist".
- Add a 0_init baseline migration that creates the base tables (the
pre-2025-12-12 shape), sorting before the first incremental migration so
the existing ALTER migrations apply cleanly on top.
- Add 20260601000000_add_login_token to create the missing magic-link
columns and index (IF NOT EXISTS, safe on installs that already have them
from a prior db push).
- Add migration_lock.toml (provider = postgresql), the standard companion to
a real migration baseline.
- Document baselining for existing databases: `migrate resolve --applied
0_init` for installs with migration history, and resolving the whole chain
for db-push installs with none (covers the Helm/Docker unattended paths).
- Add regression tests asserting the baseline exists and that every @map
column in schema.prisma stays backed by a migration (guards the drift class
that caused this bug, not just login_token).
Verified by replaying all migrations on a fresh Postgres: the chain applies
cleanly and the resulting schema matches schema.prisma exactly.
Closes#8187
* chore(plugins): re-bundle document-mode and document Stage A path
Reverts the unbundling from b0cae69ffe. Stage 0 (gzip + throttle, shipped
in 84625be849) handles size; document-mode remains opt-in per context, so
cross-context conflict risk is bounded. Stage A (keyed plugin-persistence
API, issue #7749) is the documented future path for closing the LWW gap
on different-context concurrent edits — picked up when conflicts are
observed in practice.
Design sketch with multi-reviewed phasing lives in
docs/plans/2026-05-23-stage-a-keyed-plugin-persistence.md. Predecessor
plan's "Future work" section now links to it.
* test(plugins): cover document-mode bundled load and PLUGIN_USER_DATA LWW
Follow-ups from the multi-review of 199e816479's re-bundling decision:
- E2E smoke test asserts document-mode appears in plugin management so
a typo in BUNDLED_PLUGIN_PATHS fails loudly.
- Spec exercises PLUGIN_USER_DATA conflict resolution end-to-end, which
previously relied on analogy to REMINDER (same array+null branch) but
was never directly asserted after the migration off 'virtual'.
- Stage A plan risks: stale-editor-view gap surfaced by the review;
PluginHooks.PERSISTED_DATA_UPDATE already exists in the API but is
never dispatched host-side — wiring it is the path to a fix.
- background.ts: comment marks the known gap at the registerHook site.
* docs(plugins): plan for wiring PERSISTED_DATA_CHANGED hook
Designs the host-side wiring for the currently-dead
PluginHooks.PERSISTED_DATA_CHANGED so plugins can react to remote-driven
changes to their persisted data. Multi-reviewed twice; v4 trims scope to
host-only (no plugin adoption in this design) and preserves the
multi-review insights as seeds for the follow-up doc-mode adoption
tracked at issue #7752.
Implementation lands in a separate PR.
* test: strengthen unit test assertions and revive disabled plugin specs
Replace tautological assertions, setTimeout-without-expect patterns, and
placeholder `expect(true).toBe(true)` tests with real assertions across
~30 spec files. Revive 5 plugin spec files that were fully commented out
on master with live tests covering core behavior.
Production-side: extract pure helpers for testability:
- app.component: getBackgroundOverlayOpacity, getBackgroundImageBlur
- android-sync-bridge.effects: getSuperSyncCredentialBridgeCommand
- super-sync-server: export escapeHtml, SERVER_HELMET_CONFIG
Delete the always-skipped xdescribe placeholder
src/app/imex/sync/sync-fixes.spec.ts (412 LOC).
Rename operation-log-stress.spec.ts to .benchmark.ts to match its
header comment ("excluded from regular test runs").
No production behavior changes; no master commits reverted.
Encrypt() generates a fresh random IV per call, so a retried upload of
the same logical op produces different ciphertext for the same plaintext.
The server compared encrypted payloads byte-for-byte in
isSameDuplicateOperation, mis-classifying legitimate retries as
INVALID_OP_ID instead of DUPLICATE_OPERATION. Clients only handle
DUPLICATE_OPERATION as a silent success; INVALID_OP_ID is marked as a
permanent rejection, breaking sync after a partial-success network
failure (server committed the op, client never saw the 200).
Skip payload byte-equality when both sides declare isPayloadEncrypted.
The remaining structural fields (clientId, vectorClock, clientTimestamp,
entityType/Id, actionType, schemaVersion, syncImportReason) still guard
against a UUIDv7 id collision with genuinely different content.
Both round-1 and round-2 reviewers flagged a coverage gap for the
two new behaviors most likely to silently regress on future refactors:
the WS rate-limit keyGenerator (per-(ip,clientId) keying) and the
setErrorHandler WS-429 downgrade. Closing that gap, plus adding a
real-network storm-survival test that the fake-timer unit tests
cannot model.
- Extract `wsRateLimitKeyGenerator` as a named export from
websocket.routes.ts so the (ip,cid) composition logic is reachable
from a unit test. Behavior unchanged — it was previously an inline
property on the rate-limit config object.
- Extract `pickErrorLogLevel(url, statusCode)` as a named export from
server.ts. The previous inline branch in setErrorHandler did the
same work but was untestable without booting a full Fastify app.
- Add 7 keyGenerator unit tests covering: compose, distinct keys per
cid, missing cid, regex failure, oversize cid (DoS hardening),
array-shaped query (?cid=a&cid=b), undefined query.
- Add 10 pickErrorLogLevel unit tests covering 5xx → error, the four
WS-429 path shapes (exact, trailing slash, query, both) → debug,
sibling-route over-match guard, and the 4xx-stays-loud cases.
- Add a real-network integration test that boots Fastify on a random
TCP port, connects a real `ws` incumbent + 20 challengers under
the same clientId, and asserts: every challenger receives 4008,
the incumbent stays OPEN, WARN fires exactly once, and the
storm-summary INFO fires exactly once on incumbent close.
Exercises the @fastify/websocket + ws library + sliding cooldown
path end-to-end (no DB needed, auth is mocked).
- Replace the `client.refusedChallengers = 0` post-summary reset with a
dedicated `summaryLogged: boolean` flag. The reset muddied the
field's documented "lifetime count" semantics for any future reader
observing the closing socket. The flag dedupes the `ws.on('close')`
re-entry without lying about the count.
- Refresh the stale `RECONNECT_COOLDOWN_MS` doc that still described
the non-sliding semantics from before fbdedf597e.
- Short-circuit the WS-429 path normalize on `statusCode === 429` in
setErrorHandler so the split+replace doesn't run on the 99%+ of
error responses that aren't 429.
- Drop the redundant `cid.length > 0` guard in `isValidClientId` —
the trailing regex already requires `+` (≥1 char).
- Route the websocket.routes spec simulator through `isValidClientId`
so it can't drift from production validation order again.
- Add a heartbeat-path regression test for the storm-summary dedup
(the explicit-eviction path was already covered by fbdedf597e).
The 5s cooldown introduced in ec1f09c85b kept the incumbent socket from
being evicted by a single challenger, but the gate was anchored to the
incumbent's connectedAt — so after 5s of a sustained pre-18.6.0
reconnect storm, the next challenger evicted the incumbent and the
cycle restarted every ~5s. Production logs confirmed: the same clientId
appeared in both "Reconnect within cooldown" WARNs and "Replacing stale
connection" INFOs within ~200ms.
Make the cooldown a sliding window via a single cooldownUntil field:
each refused challenger pushes it forward, so eviction only resumes
after RECONNECT_COOLDOWN_MS of quiet (no challengers). Sustained storm
keeps the incumbent forever; genuine network-blip reconnect after the
storm subsides still recovers.
Cut the per-attempt log spam:
- First refusal per incumbent emits the WARN; subsequent refusals are
silently counted on the ConnectedClient.
- On incumbent removal, log a single INFO summary with the total count
and incumbent lifetime. Zero the counter after logging so the
inevitable ws.on('close') re-entry (triggered by removeConnection's
own ws.close) cannot double-fire the summary.
- WS-route 429s downgraded to debug in setErrorHandler (storm tail, not
actionable). Exact-match on /api/sync/ws (path + query strip) so
future sibling routes do not silently inherit debug level.
Stop one bad client from draining shared-NAT quota: the WS route's
rate-limit keyGenerator now keys on \${ip}:\${clientId} instead of ip
alone. Per-IP amplification stays bounded by the server-wide 500/15min
cap. Extract isValidClientId into sync.const so the keyGenerator and
the route handler share one definition; length check runs before the
regex to reject multi-MB clientId probes cheaply.
Tests cover sliding window under sustained storm, one-WARN +
summary-on-removal, no-summary on clean close, and the
double-summary regression on the explicit-eviction path.
Pre-18.6.0 clients reconnect immediately on the 4009 eviction. With a
per-origin clientId reused across rapid reconnects, addConnection evicted
the incumbent and accepted the challenger, emitting 4009 every cycle ->
self-sustaining reconnect storm (observed ~32 evictions/sec fleet-wide,
each triggering a client download, saturating the VM).
Add a 5s reconnect cooldown: if a still-OPEN socket for the clientId was
accepted < RECONNECT_COOLDOWN_MS ago, refuse the challenger (4008) and
keep the incumbent untouched. The incumbent is never evicted, so the
server stops emitting 4009 and the loop loses its fuel. A dead/closing
incumbent bypasses the cooldown, so a genuine network-blip reconnect
still recovers.
Prisma operation.aggregate({ _min: { serverSeq } }) compiles to MIN()
over a `SELECT ... OFFSET 0` subquery. The OFFSET is a Postgres planner
optimization fence, so the (user_id, server_seq) index could not serve a
first-row seek and the query degraded to a per-user O(N) scan. Under a
client reconnect stampede this ran minutes inside the 60s interactive
download transaction, blowing the tx timeout (500s) and exhausting the
connection pool (cascading upload+download failures).
Replace it with findFirst ordered by serverSeq asc, which compiles to
ORDER BY server_seq ASC LIMIT 1 — a guaranteed index seek on the existing
unique (user_id, server_seq) index. Behaviour-preserving: same minSeq
value and same null-when-empty semantics.
Update both consuming specs (operation-download.service, gap-detection)
to the two-findFirst-call shape and add a regression test asserting the
indexed query is used and the aggregate path is not reintroduced.
- Tighten recovery guard to the idempotent drop-then-create shape (require
both DROP and CREATE INDEX CONCURRENTLY). A bare CREATE (20260511000000) is
now refused by gate, deterministically — matching its documented fail-loud
intent instead of relying on the CREATE erroring by accident. Reconciles
README with the committed migration + migration-sql.spec.ts.
- Escape single quotes in print_manual_recovery so the printed escape-hatch
commands are copy-pasteable for SQL containing quotes (e.g. the full-state
WHERE op_type IN ('SYNC_IMPORT', ...) migration).
- Fix MIGRATE_LOG temp-file leak across retry attempts.
- Add an in-script per-step timeout (with_timeout) so the Dockerfile CMD /
helm initContainer paths (no outer timeout) can't hang forever on a blocked
concurrent build; 124 fails loudly.
- Harden parse_failing_migration: sentence-anchored P3009 parse + reject
names outside the migration charset (path-traversal defence).
- Gate also accepts SQLSTATE 25001 (stable Postgres contract) not just the
localizable English message.
- MAX_ATTEMPTS -> documented tight bound (6); fail_loudly wording fixed
(was contradictory for the non-CONCURRENTLY case).
- Tests: bare-CREATE refusal, P3009 decoy-token parser hardening; trim
migration-sql.spec.ts to the architectural-invariant subset (no hardcoded
names) per 'test behavior not implementation'.
Full server suite: 36 files, 727 passed / 5 skipped.
prisma migrate deploy wraps each migration in a transaction; CONCURRENTLY
index migrations fail it (P3018/25001) and later deploys then stick (P3009).
Recovery was duplicated and migration-name-hardcoded in the host deploy.sh
and the in-image migrate-deploy.sh. The host script self-updates only via a
best-effort git pull, so a stale host deploy.sh had no recovery branch for a
new CONCURRENTLY migration and failed the deploy (the reported incident).
- migrate-deploy.sh: single, name-agnostic recovery. Parses the failing
migration from Prisma's own output, gates on the txn-block/P3009 signature
AND the migration's own SQL containing INDEX CONCURRENTLY, runs that SQL
out-of-band statement-by-statement, and only marks it applied if every
statement succeeded; otherwise fails loudly with manual steps. Bounded
retry loop; aborts instead of looping on re-failure.
- deploy.sh: ~290 lines of hardcoded host-side recovery removed; now invokes
the in-image scripts/migrate-deploy.sh (always version-locked to
prisma/migrations in the pulled image) and keeps only timeout/exit policy.
- tests: drive the script end-to-end via a fake npx (P3018, stuck P3009,
non-CONCURRENTLY refusal, statement-failure, re-failure abort, genuine
error passthrough, multi-migration chain); migration-sql.spec.ts updated
to the new contract.
- prisma/migrations/README.md: authoring rules the recovery relies on.
Design: docs/plans/2026-05-15-generic-concurrently-migration-recovery-design.md
* docs(sync): add super sync server perf plan
* perf(sync): implement supersync server perf phases
* fix(sync): bracket auth cache invalidation
* fix(sync): avoid empty replay state stringify
* fix(sync): harden supersync batch uploads
* fix super sync review findings
* fix(sync): guard payload bytes backfill rollout
* perf(sync): speed up payload_bytes backfill and index its scan
Raise the backfill batch size (DEFAULT 5->500, MAX 25->1000) so a
100M-row operations table backfills in minutes rather than tens of
hours. Add a CONCURRENTLY partial index on (user_id, id) WHERE
payload_bytes = 0: it drains to empty post-backfill so the boot-time
backfill self-check and the BOOL_OR quota probe stop doing a full
sequential scan to prove absence, and it makes the backfill's per-user
keyset paging a true index seek. Wire the new concurrent-index
migration into both deploy scripts' P3018 recovery path. Add
migration-SQL guard tests for the ADD COLUMN (metadata-only fast path)
and the new partial index.
* fix(sync): bound auth cache invalidation map and bracket every delete
The auth verification cache's invalidationVersions map grew one entry
per lifetime-invalidated user with no eviction (unbounded heap on a
long-lived single replica). Cap it at the same 10k LRU bound as the
entries map, re-inserting the just-invalidated user at the MRU tail so
the CAS race protection still holds for the only window that matters
(one DB round trip). Bracket the passkey/magic-link registration
cleanup deletes with pre+post invalidate to match the documented
convention, and invalidate on verifyEmail so a freshly-verified user
isn't denied for up to the cache TTL.
* perf(sync): skip the redundant exact replay-state measurement
The delta accounting is a proven over-estimate of the serialized state
size, so when the running bound stays within the cap the true size is
too and the final exact JSON.stringify is provably redundant. Skip it
in that case (still measure-and-throw whenever the bound does not prove
safety). This collapses the common small/incremental replay back to
zero expensive full stringifications, matching the old per-op loop
instead of regressing it. Name the entity-key JSON overhead constant
and document that assertReplayStateSize's return value is load-bearing.
* refactor(sync): split processOperationBatch into pipeline stages
Extract the 297-line batch upload method into a thin orchestrator plus
six named single-responsibility stage helpers (validate+clamp, intra-
batch dedupe, classify existing duplicates, conflict-detect, reserve
seq + insert, full-state clock). Behavior-preserving: every stage
writes terminal rejections into the shared results array by index and
the two empty-set guards short-circuit exactly as before. Also share
the timestamp clamp, the duplicate-op SELECT, and the merged
full-state clock persistence between the batch and legacy paths so
they cannot silently diverge.
* test(sync): pin batch error-code divergence and aggregate-once
Strengthen the intra-batch duplicate test to assert same-id /
different-content yields DUPLICATE_OPERATION (deliberate divergence
from the legacy INVALID_OP_ID), and document the divergence in the
plan. Replace the single-full-state aggregate test with two
full-state ops + a spy asserting _aggregatePriorVectorClock runs
exactly once and last-write-wins — the old test could not catch a
per-op-aggregate regression. Add a makeOp fixture factory. Correct
the plan's overstated replay-stringification numbers.
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: johannesjo <1456265+johannesjo@users.noreply.github.com>
A reconnecting device kept appending alongside its stale entry instead of
replacing it, so userSet would fill to MAX_CONNECTIONS_PER_USER and reject
legitimate reconnects with 4008 until the 30s+10s heartbeat cycle caught up.
addConnection now evicts any existing entry with the same clientId before
adding the new socket; the cap still applies to genuinely distinct clientIds.
The DUPLICATE_OPERATION->success conversion in the snapshot route looked
up the existing op by primary key alone. `isSameDuplicateOperation`
already enforces ownership upstream, so this can't be tripped today, but
keeping the userId filter on the route's own lookup keeps the
idempotency conversion correct even if that upstream invariant ever
changes. Switches `findUnique({id})` to `findFirst({id, userId})`;
`id` is still the primary key so the plan is unchanged.
Two correctness fixes on top of the persisted-full-state-clock optimization.
1. Persist the aggregate of prior history (merged with the snapshot op's own
clock) instead of just the snapshot op's clock. BACKUP_IMPORT uses a fresh
`{ newClient: 1 }` clock by design, so storing it raw left downloading
clients with a baseline that did not cover pre-import ops still alive in
the server-side conflict-detection set; their first post-restore edit could
be rejected as CONCURRENT. The aggregate scan moves from per-download to
per-snapshot, which is a net win — full-state ops are rare.
2. Look up the SYNC_IMPORT idempotency target by exact opId first, then fall
back to an ordered (`serverSeq DESC`) findFirst. Postgres ordering is
undefined without ORDER BY, so the previous `findFirst` could non-
deterministically return a different full-state op (e.g. a later
BACKUP_IMPORT) and flip the idempotency check incorrectly.
Two related improvements to snapshot/full-state handling:
- Persist the incoming op's vector clock as `user_sync_state.latest_full_state_vector_clock`
so downloads can skip the expensive aggregate scan when the snapshot clock is
still valid; falls back to the legacy aggregate when missing or invalid.
- Treat retried snapshot uploads (same opId) as idempotent successes instead of
409/DUPLICATE_OPERATION so a dropped response doesn't push clients into the
download-and-merge path. Per-namespace request dedup keeps ops and snapshot
caches isolated.
Previously, the storage-counter reconcile marker was set only after the
per-user drain loop finished. With the new multi-batch flow, a transient
DB failure between batch 1 and batch 2 commits batch 1's deletes but
exits before the marker call — leaving the counter stale-high until the
next daily pass or process restart, instead of self-healing on the next
sync request.
Move the marker (and affectedUserIds push) to fire after the first
successful batch so any survivor deletes are reconciled. Cover with a
test that spies the private batch helper to throw on the second call
and asserts the user is still marked.
Daily retention cleanup previously issued one unbounded `deleteMany` per user
that covered every operation older than the retention cutoff. On large
backlogs this monopolized Postgres and made user-facing sync slow during the
window, which matched the production "sync is still slow" reports.
- Cap per-batch deletes (default 5k) and per-run total deletes (default 25k),
with a select-then-delete-by-id pattern that keeps each statement short.
- Drain each user across batches before moving on so a fresh user with a tiny
backlog isn't blocked behind a large one until next pass.
- Order candidates by `snapshotAt asc` so the stalest users win the budget
when it's tight.
- Defer the initial cleanup pass after startup to 30 min in production (was
10s) so retention work doesn't compete with deploy/restart warmup; call
`unref()` on cleanup timers so they never hold the process open.
- Expose all knobs via env vars wired through docker-compose, env examples,
and the Helm chart, with a strict positive-integer parser and clamped
maxima so a misconfiguration cannot unwind the bound.
S1: replace the Fisher-Yates random shuffle in cleanup.ts with a
deterministic orderBy snapshotAt asc at the source. When the affected
user list exceeds the reconcile budget, the most-drifted users are
reconciled first; the freshest tail rolls over to the next pass. No
more Math.random, no probabilistic starvation, and the test stub for
Math.random goes away.
C1 regression tests: add coverage that proves the prototype-pollution
guard works for CRT, UPD, MOV, BATCH single-entity, and BATCH entities
map. Each test asserts state.TASK's prototype is still Object.prototype
and the malicious payload doesn't leak via the prototype chain.
C1: guard CRT/UPD/MOV/BATCH (and BATCH-entities) replay against
__proto__/constructor/prototype entityIds — bracket-assignment to those
keys invokes the __proto__ setter and swaps the entity map's prototype.
Shares the new isUnsafeEntityKey helper with the full-state branch.
W2: stop deleting storageUsageLocks[userId] in clearForUser. The
identity-guarded finally in runWithStorageUsageLock self-deletes when
the chain drains, and removing the head while a follower is queued lets
a fresh caller see no previous and race the in-flight chain. Replaces
the deprecated assertion with a race regression test.
W4: when _generateSnapshotImpl's regenerated blob exceeds the hard
MAX_SNAPSHOT_SIZE_BYTES ceiling, mirror cacheSnapshot's clear-stale
logic via a race-safe updateMany. Without this an oversize regen left
a poisoned older blob serving GET /snapshot.
W9: reject layered or non-gzip Content-Encoding values with 415 in the
content-type parser instead of falling through to JSON.parse on raw
gzip bytes (which produced a misleading 400 invalid-json).
W11: computeOpStorageBytes now returns { bytes, fallback }; route gate
and uploadOps both Logger.warn the count of unserializable ops (never
the content) so the previously-lost observability is back.
S6: drop the no-op Math.max around estimatedOpStorageBytes for
uncacheable snapshots — stateBytes is always > MAX_SNAPSHOT_SIZE_BYTES
when \!cacheable, so the floor never engaged.
The previous SYNC_IMPORT prototype-pollution regression test asserted
`Object.prototype.polluted === undefined` and `({}).polluted === undefined`.
Both assertions are tautological: `Object.assign({}, {__proto__: x})` uses
[[Set]] on the target, which triggers the `__proto__` setter only on the
target itself — it swaps `state`'s prototype reference, but does NOT
pollute the global `Object.prototype`. The old test therefore passed even
against the buggy implementation it was meant to catch.
Assert the real invariant instead:
- `Object.getPrototypeOf(result) === Object.prototype` (state's prototype
was not swapped — this is what flips under the buggy code).
- `result.polluted` is undefined via the prototype chain.
- `__proto__` is not an own property on the result.
Verified the new assertions fail against `Object.assign(state, fullState)`
and pass against the current key-by-key copy with a __proto__/constructor/
prototype skip list. Kept the global-pollution checks as defense in depth
and the `finally` cleanup in case of future regressions.
B3: remove now-redundant post-commit `applyStorageUsageDelta` writes in
the routes. Wave-1 commit 9af17e460e moved the storage-counter write
INTO `uploadOps`'s `$transaction`, so the route adding a second delta
would double-count every accepted op. /ops drops the post-commit
increment entirely; POST /snapshot now only writes `cacheResult.deltaBytes`
(the snapshot-cache portion). The route's `computeOpsStorageBytes` now
delegates to the shared `computeOpStorageBytes` helper in `sync.const.ts`
to guarantee gate-vs-increment parity.
B4: when `preparedSnapshot.cacheable === false`, do not subtract
`previousBytes` from the gate, and use `Math.max(stateBytes,
MAX_SNAPSHOT_SIZE_BYTES)` as the gate value so the worst-case op-row
write cannot slip past on a raw-JSON estimate that doesn't capture
JSONB/TOAST overhead.
B5: GET /snapshot now passes a `maxCacheBytes` cap (the user's remaining
quota) down through `generateSnapshot`. When the freshly compressed
snapshot would exceed the cap, the cache is NOT written but the
in-memory snapshot is still returned to the caller. A user already at
quota can still catch up reads without growing on-disk storage further.
Adds a regression test in `snapshot.service.spec.ts`.
B8: add `rateLimit: { max: 10, timeWindow: '15 minutes' }` to POST
/snapshot to match the other write-heavy operations (DELETE /data,
/restore/:seq). Snapshot uploads are expensive (30MB body,
`prepareSnapshotCache` zlib + JSON.stringify, full-state op replay)
so burst-uploads from one user can pin a worker.
B9: split `MAX_DECOMPRESSED_SIZE` into per-endpoint caps
(30 MB for /ops, 60 MB for /snapshot) so a 10 MB compressed body cannot
expand to 100 MB of JSON.parse work and stall the event loop.
B10: normalize `Content-Encoding` via a shared `isSingleTokenGzipEncoding`
helper so RFC-valid values like `Gzip`, ` gzip `, or `['gzip']` match.
Layered encodings (`gzip, deflate`) are rejected. Replaces all three
strict `=== 'gzip'` comparisons in `sync.routes.ts`.
Updates `sync-compressed-body.routes.spec.ts` for the new post-commit
delta contract: the snapshot route only writes the cache portion now;
the op-row portion is owned by `uploadOps`'s transaction.
Two negative-path generateSnapshot tests declared `update` on the mock tx
and asserted `expect(update).not.toHaveBeenCalled()`. Production calls
`tx.userSyncState.upsert` (not update) inside `_generateSnapshotImpl`, so
the assertion was tautological — `update` is never invoked regardless of
whether the rejection guard ran. Renaming to `upsert` makes the
"no cache write happened" assertion meaningful again.
The module-level `prisma.userSyncState.update` mock stays — production
still calls `update` from `_invalidateCachedSnapshot`, exercised by the
decompression-error test.
- B7: replace leaky `Transaction rolled back: <raw Prisma msg>` per-op
error with a generic 'Transaction failed - please retry'. The raw
exception text is still logged via `Logger.error`, but never returned
to the client (could leak SQL fragments, FK / column names).
- B6: in `deleteOldestRestorePointAndOps`, measure the exact bytes for
the deleted full-state op (SYNC_IMPORT / BACKUP_IMPORT / REPAIR) row
via `pg_column_size` BEFORE deletion. Bounded to 0-1 rows per call,
so it doesn't reintroduce the DoS from scanning every delta op. Delta
ops keep the cheap `count * APPROX_BYTES_PER_OP` approximation;
full-state payloads (up to 20MB) no longer undercount by ~20000x and
leave the counter permanently low if reconcile fails. Docstring on
APPROX_BYTES_PER_OP clarifies it is ONLY valid for delta cleanup.
- I6: inside `deleteOldSyncedOpsForAllUsers`, call
`storageQuotaService.markNeedsReconcile(userId)` for each user whose
cleanup deleted rows. A crash mid-loop now leaves surviving users
with a forced-reconcile marker so their next request self-heals,
instead of waiting for the daily pass. TODO left for persisting the
marker in a DB column for cross-restart / multi-instance safety.
- B3 (service side): write `users.storage_used_bytes` atomically
inside the upload `$transaction` using `$executeRaw` with the same
GREATEST(...) clamp pattern as `storage-quota.service.ts`. Shared
`computeOpStorageBytes` helper keeps the gate and the increment in
agreement about per-op size. Clean-slate path SETs the counter to
the new total rather than incrementing onto the just-reset row.
- B2 companion: add `RequestDeduplicationService.clearForUser(userId)`
and call it from both `deleteAllUserData` and the uploadOps
clean-slate path so a retry from before the wipe cannot return
cached results that reference now-deleted state.
The minSeq-based gap check compared the raw sinceSeq, so a client
requesting from before retention purge would get gapDetected=true
even when the snapshot already supersedes that purged history. Case 3
already used effectiveSinceSeq; this aligns Case 2 so the snapshot
fast-forward suppresses the false positive and avoids triggering an
unnecessary full restore on the client.
The deferred-reconcile budget caps at 720 users per pass
(RECONCILE_BUDGET_MS / RECONCILE_INTERVAL_MS = 3.6M / 5K). With a stable
DB iteration order, the first 720 affectedUserIds got reconciled every
daily cleanup and any tail beyond that drifted indefinitely.
Shuffle the affected-user list with Fisher-Yates before slicing the
budget window so each pass picks a different random subset, and include
both numbers in the warn log so operators can spot servers that have
outgrown the per-pass budget.
clearForUser now also clears forcedReconciles (would force a spurious
extra scan on the next quota check) and storageUsageLocks (orphaned
queued lock chain) alongside the existing inflightReconciles cleanup.
Also document the known byte-counting mismatch between pg_column_size
(TOAST-compressed) and the hot-path Buffer.byteLength(JSON.stringify)
in calculateStorageUsage. The proper fix needs a payload_bytes column
populated at insert time; in-place switching to octet_length(text)
would re-detoast every row and resurrect the disk-I/O DoS.
