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.
Address three review findings on snapshot.service.ts:
- B1: _generateSnapshotImpl no longer unconditionally upserts the cache.
A concurrent upload that bumped lastSnapshotSeq beyond our latestSeq
could be clobbered. Switched to the same race-safe updateMany+create
pattern as cacheSnapshot, and pin cacheDelta to 0 when the race is
lost so the onCacheDelta hook does not over-credit storage.
- I2: Always run _assertCachedSnapshotBaseReplayable before serving a
cached snapshot, including the startSeq>=latestSeq fast path.
A legacy build that failed to reject encrypted ops could have
produced a poisoned cache that the fast path would serve forever.
- I4: replayOpsToState no longer uses Object.assign for full-state op
payloads. A malicious SYNC_IMPORT/BACKUP_IMPORT/REPAIR payload with
a __proto__ key would pollute Object.prototype via the [[Set]]
semantics of Object.assign. Copy keys explicitly and skip
__proto__/constructor/prototype.
I3 (RepeatableRead transaction spans gzip/replay/gzip and can starve
the DB pool under load) is acknowledged but left for a focused PR —
restructuring requires moving the cache write outside the transaction
and updating multiple integration tests.
Updated unit-test mocks to provide updateMany/create on the
transaction client, and added regression coverage for the prototype-
pollution and fast-path-validation cases.
- B11: Remove orphan deploy-db-scalar.mjs and dead RUN_POST_MIGRATION_INDEXES
references in env.example and README. Rewrite README deploy section to
match what scripts/deploy.sh actually does (drop false claims about
empty migration SQL, failed-migration recovery, and optional index builds).
- B12: Default RUN_MIGRATIONS_ON_STARTUP to false in docker-compose.yml so
the compose, Dockerfile ENV, and env.example agree on the safer default
(manual deploy.sh runs migrations once before app restart). Update the
migration-sql.spec assertion to match.
- I9: Wrap prisma migrate deploy in deploy.sh with a MIGRATION_TIMEOUT
(default 900s). CREATE INDEX CONCURRENTLY can block on long-running
transactions; on timeout (exit 124) the script now fails loudly with a
clear error rather than hanging.
Deadlock: updateStorageUsage now bypasses the inflightReconciles dedupe
when already inside the per-user lock, so a reentrant caller cannot
await a non-reentrant promise queued behind the lock it holds.
Counter drift: GET /snapshot's cache rewrite feeds its byte delta into
the storage-usage counter via an onCacheDelta hook; failed counter
writes mark the user for forced reconcile on next quota check;
oversized snapshots clear the stale row and credit the freed bytes.
Event loop: zlib gzip/gunzip moved off the synchronous fast path so
large snapshots stop blocking other tenants.
Hardening: /ops bodyLimit set to MAX_COMPRESSED_SIZE_OPS; snapshot
pre-gate reconciles before rejecting a stale-high counter; all 413
responses funnel through one helper using errorCode consistently;
reconcileTimers leak fixed by self-removing fired timers.
Includes regression tests for each new behavior.
- Use upsert for userSyncState to avoid RecordNotFound on first-time users
- Reset (not merge) state on full-state ops; throw on malformed payloads
- Add typed EncryptedOpsNotSupportedError; stop leaking op count to clients
- Split JSON.parse vs decompress error paths (new invalid-json reason)
- Measure base64 transport size against decoded binary; decode once
- Guard concurrent cacheSnapshot writes against stale overwrite
- Move encryption-skip policy from route into cacheSnapshotIfReplayable
- Invalidate corrupt cached snapshot blobs instead of re-logging forever
- Use Buffer.byteLength for replay state size (was UTF-16 vs byte limit)
- Skip cache-replayability assert on the cache-hit fast-path
- Extract async gunzip/gzip into gzip.ts; stop blocking the event loop
- Replace inline tx structural types with Prisma.TransactionClient
- deploy.sh: set -euo pipefail with guarded vars and pipeline-safe diagnostics
- Accept leading-gap replays only when first surviving op is a full-state op
(SYNC_IMPORT / BACKUP_IMPORT / REPAIR); fixes SNAPSHOT_REPLAY_INCOMPLETE after
clean-slate uploads and retention pruning. Mid-stream gaps still throw.
- Snapshot transactions now run at RepeatableRead so encrypted-op guards
cannot be raced by a concurrent writer.
- Restore getParsedBodySizeBytes for storage-quota accounting: JSON.stringify
.length counts UTF-16 code units and re-serializes the payload; prefer the
parser-reported decompressed byte count, then Content-Length, then a
Buffer.byteLength fallback.
- Extend in-memory tx mocks with count + opType-aware findFirst so the new
encrypted-op guards exercise cleanly under vitest.
