feat(core): lock a content entry while someone is editing it - #2919
Draft
danielmlr wants to merge 3 commits into
Draft
feat(core): lock a content entry while someone is editing it#2919danielmlr wants to merge 3 commits into
danielmlr wants to merge 3 commits into
Conversation
Two people can open the same entry today and neither is told the other is there. emdash-cms#2126 made a stale save return 409 instead of silently replacing a newer draft, but that reports the collision after both people have done the work, and the only resolution left is to discard one side. An entry now takes a lease when it is opened, keyed on (collection, entry_id) in a new system table. The claim is a single INSERT ... ON CONFLICT DO UPDATE ... WHERE statement rather than a read-then-write, so two editors racing on one entry cannot both win. Timestamps come from the database clock in the format the media-usage lease already uses, so leases stay comparable across isolates whose wall clocks drift. The lease survives a save. Released on save it would be gone two seconds after the next pause, because autosave debounces at AUTOSAVE_DELAY, so the refresh rides on the autosave PUT the admin already sends: one statement, no extra request, and nothing added to the logged-out path. A write against someone else's live lease is refused with 409 ENTRY_LOCKED unless the caller opts out. The refusal carries the holder, which is how the previous holder learns their entry was taken: their next save is refused and the editor says so rather than failing quietly. ENTRY_LOCKED is a new code rather than the existing WORK_LEASE_ACTIVE because mediaUsageWorkRetryConflictSchema pins that code's details to { leaseExpiresAt } and is wired into the OpenAPI document, so reusing it would make the published spec claim one code carries two incompatible shapes. Locking is on for every collection and switches off with edit_locking on _emdash_collections. The MCP content tools do not honour the lock yet; that lands separately.
The lock upsert failed on Postgres: inside ON CONFLICT DO UPDATE the stored row and `excluded` both carry `user_id`, so a bare column name is ambiguous there. Every acquire returned ENTRY_LOCK_ERROR and the admin fell back to editing unlocked. The conflict branch now qualifies its references. Nothing renewed an open tab's lease, so a holder who paused for seven minutes lost the entry without knowing, and the next colleague acquired it without a take-over prompt. The admin now re-asserts the lease every two minutes, re-checks it when the tab becomes visible again, and reports a lost lease itself; a reader is moved to editing once the holder leaves. A take-over therefore reaches the previous holder within one heartbeat. The lease was keyed on the account, so closing one of two tabs of the same user dropped the lease the other still relied on. Each mounted editor now sends a per-tab token; the row keeps the latest one and a release matches on it. Closing the tab releases through a keepalive request, and a grant that lands after the editor moved on is handed back. `/schedule` on POST and DELETE now refuses a write against someone else's lease like the other write routes, with the same opt-out on body and query string, carried through the client and the CLI. The refusal message names the holder, the OpenAPI document describes the 409 body on every refusing route, and the CLI flag is declared as `override-lock` so `--help` prints what the docs say. A refused autosave is resent once the entry is taken back. `editLocking` only enters the seed capture fingerprint when it is off, so existing collections keep theirs. Tests cover two collections sharing an entry id, the token, the heartbeat, the visibility re-check, the keepalive release and the in-flight hand-back.
The note that the edit lock does not refuse MCP writes sat under content_update alone, so the sections for content_delete, content_publish, content_unpublish, content_schedule, content_unschedule and content_discard_draft read as if they honoured it. None does: no content write tool passes through claimEntryLockForWrite. The sentence now opens the Content Tools section and links the lock's reference. Also drops the comment on entryLockConflictSchema that explained why it is not registered as a component; the reason it gave does not hold, and the sibling media-usage conflict schema is inlined without one.
🦋 Changeset detectedLatest commit: ec78032 The changes in this PR will be included in the next version bump. This PR includes changesets to release 17 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
Scope checkThis PR changes 3,518 lines across 58 files. Large PRs are harder to review and more likely to be closed without review. If this scope is intentional, no action needed. A maintainer will review it. If not, please consider splitting this into smaller PRs. See CONTRIBUTING.md for contribution guidelines. |
@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-moderation
@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: |
17 tasks
Contributor
Overlapping PRsThis PR modifies files that are also changed by other open PRs:
This may cause merge conflicts or duplicated work. A maintainer will coordinate. |
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?
Prevents two editors from colliding on one entry, rather than reporting the collision once both have done the work. #2126 turns a stale save into a 409, but that arrives after the typing is done, and the only resolution left is to discard one side.
Opening an entry in the admin now takes a lease on it, one row per
(collection, entry_id), claimed in a single insert-or-steal statement so two editors racing on one entry cannot both win. The second editor is told who holds it and chooses read-only or take-over; the holder learns of a take-over on their next heartbeat or save, and their editor turns read-only with a banner naming who has it now.PUT,DELETE,/publish,/unpublish,/scheduleand/discard-draftrefuse a write against someone else's live lease with409 ENTRY_LOCKEDand name the holder;overrideLockopts out, and the CLI exposes--override-lock. The lease runs seven minutes; the admin re-asserts it every two minutes and on every save, and hands it back when the editor is left or the tab closes. Nothing is added to the logged-out path. Locking is on for every collection and switches off per collection.Design and defaults are discussion #2819; ascorbic approved the split it proposes,
_revon the MCP write tools first and locking on top, in Discord.Four things carry over from that thread:
ENTRY_LOCKED, not theWORK_LEASE_ACTIVEthe discussion named.mediaUsageWorkRetryConflictSchemapins that code'sdetailsto{ leaseExpiresAt }and is wired into the OpenAPI document at both media-usage retry paths, so reuse would make the published spec claim one code carries two incompatible shapes.syncNonTranslatableFieldswrites sibling locales with raw SQL and bypasses the lock exactly as it bypasses_rev. That is Non-translatable fields never sync to sibling locales on collections that support revisions #2911, inherited rather than fixed here.075_entry_edit_lockstakes the next free number onmain. Published migrations are immutable, so another 075 landing first means renumbering this one.Type of change
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (or targeted tests for my change)pnpm formathas been runmessages.pochanges except in translation PRs — a workflow extracts catalogs on merge tomain.AI-generated code disclosure
Screenshots / test output
The refusal path is covered at the repository, at the handlers and at every refusing route, against real databases; the repository suite runs on Postgres too, in CI, where the integration job has one. The two-editor journey has no end-to-end test: the e2e fixture has a single identity, dev-bypass is hard-wired to
dev@emdash.local, and a second live session needs the invite and virtual-authenticator flow. So the end-to-end tests cover the lease mechanics with that one identity, and the 409-to-banner seam is covered at the hook level.