feat(collab-doc): If-Match optimistic concurrency so persist never clobbers an out-of-band edit by waleedlatif1 · Pull Request #6085 · simstudioai/sim

waleedlatif1 · 2026-07-30T02:31:09Z

Summary

Close the last collab-doc data-safety residual: the relay's persist could clobber a durable write it hadn't seen. Adds RFC 7232 If-Match optimistic concurrency end-to-end, reconciling through the CRDT so it never rejects user work.

The relay projected the live Yjs doc back to markdown unconditionally (last-write-wins). #6070 routes external writes into the live doc, so this is now a narrow race — a persist already in flight when an external write lands — but it was real (and on a last-leave flush there was no "next persist" to heal it).

How it works

updateWorkspaceFileContent gains an expectedUpdatedAt guard — the write commits only if the file is still at that version, checked against the SELECT … FOR UPDATE-locked row (atomic with the write); else it throws the new ContentVersionConflictError without clobbering.
persistFileDoc takes expectedVersion and returns a discriminated result (persisted | missing | conflict); on conflict it returns the current durable content + version instead of writing.
The relay tracks the durable version its live doc is synced to — set on seed, advanced when a durable write is merged in (apply-edit carries the version), and on each successful persist. Held cluster-wide in Redis (filedoc:syncver:{name}) so whichever task runs a persist reads the same version (per-room value is the single-pod fallback) — this is what keeps it correct on multi-task ECS.
flushPersist sends that version as If-Match. On a conflict it merges the current durable content into the live doc (so the out-of-band edit and the live edits converge) and retries (bounded) — so even a last-leave flush racing an external write persists the reconciled result rather than losing the session's edits.

Notes

No schema change — reuses workspace_files.updatedAt as the version token.
Honest limitation (unchanged, architectural): a truly lossless 3-way merge of a full-markdown out-of-band write vs live CRDT edits isn't possible without operational diffs. The guarantee here is no silent clobber + reconcile-through-the-CRDT; overlapping-region resolution stays "incoming wins" (same as copilot today).

Type of Change

Bug fix (data safety)

Testing

App CAS: matching guard writes; mismatch throws ContentVersionConflictError and cleans up the staged upload without advancing the row.
Relay: a persistent conflict is handled gracefully (attempted, durable left authoritative, no clobber, no unbounded loop).
Full gates: sim + realtime tsc, lint, api-validation, boundaries, prune; 236 realtime + 76 sim collab/uploads tests.
Live check owed: two editors + a copilot write racing a persist; and a last-leave flush during an external write.

Checklist

Code follows project style guidelines
Self-reviewed my changes
Tests added/updated and passing
No new warnings introduced
I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

…ers an out-of-band edit The relay projected the live Yjs doc back to durable markdown unconditionally (last-write-wins), so a persist already in flight when an external write landed could overwrite it. Add RFC 7232 If-Match optimistic concurrency end to end, reconciling through the CRDT (never rejecting user work): - updateWorkspaceFileContent gains an expectedUpdatedAt guard: the write commits only if the file is still at that version (checked against the SELECT ... FOR UPDATE-locked row, so it is atomic with the write), else it throws the new ContentVersionConflictError without clobbering. - persistFileDoc takes expectedVersion and returns a discriminated result (persisted | missing | conflict). On conflict it returns the current durable content + version instead of writing. - The relay tracks the durable version its live doc is synced to — set on seed, advanced when a durable write is merged in (apply-edit carries the version), and on each successful persist. It is held cluster-wide in Redis (filedoc:syncver:{name}) so whichever task persists reads the same version, with the per-room value as the single-pod fallback. - flushPersist sends that version as If-Match. On a conflict it merges the current durable content into the live doc (so the out-of-band edit AND the live edits converge) and retries (bounded), so even a last-leave flush racing an external write persists the reconciled result rather than losing the session's edits. Threads the version through the seed + persist contracts and the apply-edit payload. No schema change (reuses workspace_files.updatedAt as the version token). Tests: app-side CAS (match writes, mismatch throws + cleans up the orphan upload), relay conflict handled gracefully without clobber/loop; 236 realtime + 76 sim collab/uploads tests, tsc x2, lint, api-validation, boundaries, prune all green.

vercel · 2026-07-30T02:31:15Z

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
docs	Skipped		Jul 30, 2026 7:47am

cursor · 2026-07-30T02:31:19Z

PR Summary

High Risk
Touches collaborative persistence, multi-task Redis version tokens, and workspace file CAS—core data-safety paths where regressions could lose or overwrite edits.

Overview
Adds optimistic concurrency so relay persist cannot silently overwrite durable markdown when the file changed out-of-band (copilot/tools) while a flush was in flight.

Durable layer: New workspace_files.content_updated_at is the If-Match token (content writes only; renames do not bump it). updateWorkspaceFileContent accepts expectedUpdatedAt, compares it under FOR UPDATE, throws ContentVersionConflictError on mismatch, and stamps strictly monotonic contentUpdatedAt. External markdown writes pass that version into mergeEditIntoLiveFileDoc / apply-edit so the live doc is marked as already incorporating that durable revision.

App persist API: Seed returns { update, version }. persistFileDoc takes expectedVersion and returns persisted | missing | conflict | deferred instead of a boolean; conflicts do not write.

Realtime relay: Tracks syncedVersion per room and cluster-wide in Redis (filedoc:syncver, monotonic Lua set). flushPersist projects stream/local state with expectedVersion; on conflict it makes one attempt and stops (no re-persist against a possibly-behind stream)—durable stays authoritative until the write chokepoint merge lands. Merge lock failure returns merge-unavailable (distinct from no-live-room).

^{Reviewed by Cursor Bugbot for commit df92969. Configure here.}

…side its stream Keep filedoc:syncver:{name} alive as long as the room's stream (it was only re-set on seed/merge/persist), so an open-but-idle doc's persist If-Match token can't expire and force a needless reconcile.

waleedlatif1 · 2026-07-30T02:34:39Z

@greptile

waleedlatif1 · 2026-07-30T02:34:40Z

@cursor review

greptile-apps · 2026-07-30T02:34:59Z

Greptile Summary

Adds RFC 7232 If-Match optimistic concurrency so collab-doc persist cannot silently clobber out-of-band durable writes.

updateWorkspaceFileContent / persistFileDoc CAS on content version and return persisted | missing | conflict | deferred.
Relay tracks cluster-wide synced version (Redis + room), stamps it only after a winning seed or successful merge/persist, and stops on conflict without re-persisting stale stream state.
Last-leave and multi-task paths leave durable content authoritative on conflict; deferred when the version token is unavailable.

Confidence Score: 5/5

Safe to merge; prior last-leave clobber, seed version-skip, and pre-win stamp issues are resolved at HEAD with no remaining blocking failures.

Conflict handling is single-attempt stop with durable left authoritative; seed version is written only after seedIfEmpty wins; persist returns content version and defers when the token is missing; no outstanding prior defects remain on the changed paths.

Important Files Changed

Filename	Overview
apps/realtime/src/handlers/file-doc.ts	If-Match flush, seed stamp-after-win, conflict stop without stale retry; prior last-leave clobber/seed-skip paths closed.
apps/realtime/src/handlers/file-doc-store.ts	Cluster syncver get/set with monotonic Lua and TTL heartbeat alongside stream.
apps/sim/lib/collab-doc/persist.ts	Discriminated persist result; defers without version; returns content version on persist/conflict.
apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts	expectedUpdatedAt CAS under SELECT FOR UPDATE; ContentVersionConflictError without clobber.
packages/db/schema.ts	content_updated_at (and related migration) as content-scoped version token separate from metadata updatedAt.

_{Reviews (18): Last reviewed commit: "fix(collab-doc): stop (don't re-persist)..." | Re-trigger Greptile}

…doc to reconcile On an If-Match conflict with no live doc to reconcile into (last collaborator gone, no shared stream), applyMarkdownToLiveFileDoc returns no-live-room; re-projecting the same pre-teardown snapshot would only re-conflict, so break the retry loop immediately and leave the out-of-band (durable) content authoritative — the intended conflict policy. Addresses Greptile review.

waleedlatif1 · 2026-07-30T02:43:16Z

@greptile

waleedlatif1 · 2026-07-30T02:43:17Z

@cursor review

…eview - Single-pod persist retry projected the pre-reconcile snapshot (captureState always returned the initial localState), while the synced version had been advanced by the reconcile — so the If-Match could pass and clobber the reconciled edit. captureState now re-reads the live doc on each attempt (falling back to the pre-teardown snapshot only once the room is gone). - The synced version was recorded from this task's own seed FETCH before knowing whether this task's seed actually won; a peer winning with a different version could leave a newer token than the stream content. Record it only inside the didSeed branch (the task whose seed won); peer-seeded tasks read the winner's cluster value. - Persist wrote UNCONDITIONALLY when no version was available (relay version momentarily missing), which could clobber non-empty durable content. It now returns conflict for a non-empty file with no version (reconcile/retry once the version is re-established); an empty file's first write stays unconditional.

waleedlatif1 · 2026-07-30T02:56:31Z

@greptile

waleedlatif1 · 2026-07-30T02:56:32Z

@cursor review

…e freshest version token - Missing-version persist now returns 'deferred' instead of 'conflict'. A missing version token (a Redis blip on a peer-seeded task) is NOT a genuine out-of-band change, so triggering a reconcile would wipe live edits (incoming-wins) even though nothing changed durably. Deferred means: don't write, don't reconcile — leave the edits in the stream and let a later persist write them once the version is re-established. - currentVersion now takes the MAX of the cluster (Redis) and local room versions rather than always preferring Redis, so a lagged/failed fire-and-forget Redis set can't shadow a newer local value and cause spurious If-Match conflicts. Versions are monotonic epoch-ms, so the larger is the later sync.

waleedlatif1 · 2026-07-30T03:09:30Z

@greptile

waleedlatif1 · 2026-07-30T03:09:31Z

@cursor review

…er missing version on final flush Close two last-leave concurrency holes Cursor flagged: - Thread the reconciled version LOCALLY through the persist retry loop. After a conflict+reconcile the correct next If-Match is exactly result.version, so carry it in a local var instead of re-deriving from room.syncedVersion/Redis. On a last-leave flush destroyRoomIfIdle removes the room from the map before the async flush finishes, so mergeMarkdownIntoRoom's recordVersion can no longer update room.syncedVersion — threading makes each retry's precondition correct by construction, immune to that dropped mutation and to a best-effort Redis re-read. - Cache the resolved version back into room.syncedVersion in currentVersion() so a peer-seeded/tail-only task (which never sets it locally) or a later transient Redis read failure still resolves it from the last value seen (monotonic max, never regresses). - On a FINAL flush, briefly retry resolving the If-Match when the version read momentarily fails, rather than deferring and stranding the session's edits in the TTL'd stream — the version is cluster-wide and heartbeat-refreshed.

waleedlatif1 · 2026-07-30T03:27:59Z

@greptile

waleedlatif1 · 2026-07-30T03:28:00Z

@cursor review

waleedlatif1 · 2026-07-30T05:06:35Z

@cursor review

…ects (would wipe newer edits) flushPersist reconciled the durable body into the live doc on every conflict. But when the conflict comes from a racing self-persist (or an apply-edit the chokepoint already merged), the durable body is a STALE SUBSET of the live stream, and the incoming-wins updateYFragment merge moves the doc backward — wiping newer in-flight edits, which the retry then persists. Before reconciling, re-check the freshest synced version. If it already covers the conflict version, the live doc has already incorporated that content (or is ahead), so skip the reconcile and just retry with the freshest version as If-Match — the re-projection captures the current live stream, preserving every edit. Only a genuine out-of-band change the live doc hasn't incorporated (freshest < conflict version) is reconciled in. freshest never exceeds the durable version, so this can't loop.

waleedlatif1 · 2026-07-30T05:20:28Z

@greptile

waleedlatif1 · 2026-07-30T05:20:29Z

@cursor review

…oncile can't loop The If-Match token was stamped with app-local new Date() on each content write, so cross-instance clock skew could stamp a later write with an EARLIER content_updated_at — breaking the version ordering the whole optimistic-concurrency scheme (and the skip-reconcile branch's freshest>=version assumption) depends on. Under skew the relay's monotonic syncedVersion could exceed the durable version, sticking the If-Match: persist conflicts forever, exhausts retries, drops the session's edits. - Stamp content_updated_at strictly after the current committed value (we hold the row's FOR UPDATE lock): new Date(max(now, currentFile.contentUpdatedAt + 1ms)). Monotonic per file regardless of clocks; also removes same-millisecond collisions. updatedAt stays plain wall-clock (display/sort). - Skip-reconcile branch retries with result.version (the durable value the CAS will match), never freshest (which could exceed it and loop). Belt-and-suspenders now that the version is monotonic.

waleedlatif1 · 2026-07-30T05:34:26Z

@greptile

waleedlatif1 · 2026-07-30T05:34:27Z

@cursor review

…t-version-and-retry on conflict The in-persist reconcile projected the durable body back over the live doc via updateYFragment ("make the doc match"). That is destructive: when the live stream is already ahead — the common case, because the write chokepoint (mergeEditIntoLiveFileDoc) already merged the out-of-band change into the stream — it moved the doc backward and wiped newer in-flight edits. This produced a run of races (stale snapshot, wipe-newer-edits, version-lag skip miss) that a full-document reconcile fundamentally can't avoid, since deciding when it's safe relies on a laggy cross-task version token. Remove it. On conflict, adopt the durable version as the new If-Match and retry: captureState re-reads the current stream (which holds the out-of-band change AND the live edits), so the re-projection persists the converged result. The durable change reaches the live doc via the chokepoint, never here. Trade-off: the only unmerged out-of-band write is one whose chokepoint merge itself failed (rare, logged), which we accept over the frequent reconcile-wipes-edits race. - flushPersist: conflict -> ifMatch = result.version, retry (bounded). No applyMarkdownToLiveFileDoc. - conflict response drops `markdown` (contract + relay type + persist) — no body needed, saves a blob fetch. applyMarkdownToLiveFileDoc stays (still used by the apply-edit route / the chokepoint).

waleedlatif1 · 2026-07-30T07:22:55Z

@greptile

waleedlatif1 · 2026-07-30T07:22:56Z

@cursor review

cursor

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit f965bc1. Configure here.}

…e stale local snapshot Regression from dropping the reconcile: on conflict the retry adopts result.version and re-reads captureState. But after single-pod last-leave teardown the room is already destroyed, so captureState falls back to the pre-teardown localState (which lacks the out-of-band change); the retry then CAS-passes and overwrites the committed external write — undoing the external-wins last-leave policy. Null localState on the first conflict, so the retry can only use freshly-read authoritative state (stream / live doc). When none is available (single-pod room gone, or a transient stream-read failure) captureState returns null and the retry stops, leaving durable content authoritative. Covers both the single-pod and multi-task-stream-unavailable variants of the stale-snapshot clobber.

waleedlatif1 · 2026-07-30T07:31:04Z

@greptile

waleedlatif1 · 2026-07-30T07:31:05Z

@cursor review

…es the commit-window clobber The conflict retry adopted the durable version and immediately re-persisted the current stream, assuming the stream already held the out-of-band change. But an external write commits durable BEFORE its chokepoint merge (mergeEditIntoLiveFileDoc) reaches the stream, so a persist landing in that window CAS-passed with a stream that still lacked the external content and clobbered the committed write — not just the rare merge-failed path, but a race on every external write, worst at last-leave flushes. Make persist a single attempt: on conflict, STOP and leave durable authoritative. The chokepoint merges the change into the stream and — only once it is actually there — advances the synced version via its own recordVersion; a later flush (debounced or final) then projects the converged stream with a matching token. The session's edits stay in the stream meanwhile. The conflict handler deliberately does NOT advance the synced version, or the next flush would clobber with a still-behind stream. Removes the retry loop and PERSIST_CONFLICT_RETRIES.

waleedlatif1 · 2026-07-30T07:47:40Z

@greptile

waleedlatif1 · 2026-07-30T07:47:41Z

@cursor review

cursor

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit df92969. Configure here.}

vercel Bot temporarily deployed to Preview July 30, 2026 02:34 Inactive

greptile-apps Bot reviewed Jul 30, 2026

View reviewed changes

Comment thread apps/realtime/src/handlers/file-doc.ts Outdated

Comment thread apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts Outdated

greptile-apps Bot reviewed Jul 30, 2026

View reviewed changes

Comment thread apps/realtime/src/handlers/file-doc.ts Outdated

vercel Bot temporarily deployed to Preview July 30, 2026 02:42 Inactive