fix(core): CAS-guard the draft-revision pointer flip on concurrent saves#2130
Draft
marcusbellamyshaw-cell wants to merge 2 commits into
Draft
Conversation
Two saves to a collection with no draft yet (autosave + explicit save, or two concurrent explicit saves) both read draft_revision_id: null, then race an unconditional UPDATE to flip the pointer. Whichever lands last wins regardless of which read the fresher base data, so the other save's edit is silently discarded even though the API reports success. D1 has no multi-statement transactions, so the pointer-flip UPDATE is now a CAS keyed on the draft_revision_id read at the start of the attempt; a losing save retries against the fresh pointer (re-merging its edit on top) instead of clobbering it. Reported as a comment on emdash-cms#1158 (the original TipTap-dispatch cause of emdash-cms#1158 itself was already fixed by emdash-cms#1119; this addresses the separate save-race still reproducing on 0.28.1/0.29.0). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: f501b03 The changes in this PR will be included in the next version bump. This PR includes changesets to release 16 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
|
Could not push formatting changes to this fork. The contributor may have "Allow edits by maintainers" disabled. Please run the formatter locally: |
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/auth-atproto
@emdash-cms/blocks
@emdash-cms/cloudflare
@emdash-cms/contentful-to-portable-text
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@emdash-cms/plugin-cli
@emdash-cms/plugin-types
@emdash-cms/registry-client
@emdash-cms/registry-lexicons
@emdash-cms/registry-verification
@emdash-cms/sandbox-workerd
@emdash-cms/x402
@emdash-cms/plugin-ai-moderation
@emdash-cms/plugin-atproto
@emdash-cms/plugin-audit-log
@emdash-cms/plugin-color
@emdash-cms/plugin-embeds
@emdash-cms/plugin-field-kit
@emdash-cms/plugin-forms
@emdash-cms/plugin-webhook-notifier
commit: |
CI format:check flagged this file after the PR emdash-cms#2130 diff shifted line wrapping. Reformatted with oxfmt, no logic change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Fixes a save race reported as a follow-up comment on closed issue #1158.
EmDashRuntime.handleContentUpdate's draft-revision path does read-existing → merge → create-revision → flip-pointer with no wrapping transaction (D1 has no multi-statement transactions) and an unconditional pointer-flipUPDATE. Two concurrent saves (autosave racing an explicit save, or two explicit saves) that both readdraft_revision_id: nullbefore either writes both create a new revision and race to flip the pointer — whicheverUPDATElands last wins regardless of which read the fresher data, so the other save's edit is silently discarded even though the API reports success. This matches the DCathal repro in the #1158 thread (two revisions created in the same second, the stale one becomesdraft_revision_id).Note: the original #1158 report (TipTap
editor.view.dispatchbypassingonUpdate) was already fixed by #1119 and is unaffected here — this addresses the separate, still-reproducing save race described in the newest comment on that thread (0.28.1/0.29.0).Fix: the pointer-flip
UPDATEis now a compare-and-set keyed on thedraft_revision_idread at the start of the attempt (COALESCE(draft_revision_id, '') = <value read>, NULL-safe across SQLite/D1 and Postgres). If the CAS fails, the losing save retries — re-reading the now-current draft, re-merging its own edit on top, and retrying the create+CAS — instead of silently overwriting. Bounded to 3 attempts. The orphaned revision row from a lost race is harmless;pruneOldRevisionssweeps it up on a later successful save since it's keyed byentry_id, not by pointer.Known residual, out of scope for this PR: the autosave-only "update existing draft revision in place" path (
skipRevisionwhen a draft already exists) still isn't CAS-guarded — two concurrent autosaves (no explicit save involved) can still last-write-wins on that single row'sdatacolumn. Closing that fully would need a version column onrevisions, which felt like more surface than this fix needs; flagging in case a maintainer disagrees.Closes (comment thread on) #1158
Type of change
Checklist
pnpm typecheckpasses (pre-existing unrelated errors inoauth-state-store.ts/plugins/context.tsconfirmed present without this change)pnpm lintpasses (oxlint, scoped to changed files)pnpm testpasses (targeted: new race test + adjacent content/revision/media-usage integration suites, both sqlite and postgres dialects)pnpm formathas been run (skipped locally — CRLF checkout makesformat:checkunreliable on Windows; will check CI)emdash)AI-generated code disclosure
Test plan
Added
packages/core/tests/integration/database/content-update-save-race.test.ts: creates a published entry, then fires two concurrenthandleContentUpdatecalls editing different fields, with aRevisionRepository.createspy that gates the first call until the second also arrives — forcing both to readdraft_revision_id: nullbefore either writes, regardless of driver timing. Verified the test fails against the pre-fix code (save B's edit silently dropped, matching the report) and passes with the fix (both edits survive).