fix(decofile): stop stale editor copies from reverting other drafts' block changes

Merged
VI
3d ago
+1607-89

What is this contribution about?

In Fast Preview projects, one draft's publish can silently undo another draft's edits.

The problem

Every CMS save replaces a whole block. The payload is the copy the editor holds plus the field the user just changed. Nothing checks whether that copy is still what the branch has.

Here is the sequence we traced in a production project. Two editors were on separate drafts and both edited the same global header block:

  1. Editor A publishes a change to the header menu.
  2. Editor B's draft has no commits of its own yet, so the header's automatic "get latest" fast-forwards it onto production. B's draft now contains A's change.
  3. B's section form for the header is still open. Its formValue was seeded when the section was selected, and a refetch of the decofile does not re-seed it.
  4. B changes one field. The autosave sends the whole stale block. The commit lands on top of A's change and reverts it.
  5. B publishes, and production loses A's menu change. The diff of B's publish is the exact inverse of A's diff, plus B's one-field change.

There is a second, rarer path. When the sync before a publish hits a git conflict, repoGitRebase lets the branch's whole file win for every path it touched. That also drops the base's edits in parts of the file the branch never changed.

The fix

1. The server refuses stale writes (commit-coalescer.ts, routes/decofile.ts)

  • PATCH /api/:org/decofile/:vmcp/:branch accepts an optional expect: { [blockKey]: fingerprint | null }. The fingerprint is a SHA-256 of the block's JSON with keys sorted and secrets sanitized (decoBlockFingerprint in @decocms/shared/decofile). null means "I saw no such block".
  • The coalescer checks each patch's expectations against the head it is about to commit on. Within a coalesced batch it checks them in arrival order, so a later patch is compared with what the earlier ones leave behind. A stale patch is rejected with 409 { code: "block-conflict", blocks } and none of it is written; the other patches in the batch still land.
  • Requests without expect behave exactly as before.

2. The editor says what it edited from (use-save-block.ts, sections-editor.tsx, form-base.ts)

  • useSaveBlock sends the fingerprint of the cached block at call time. That is the right base for every write built from the cache.
  • An open section form now remembers the section value it started from. It updates that value after each of its own writes and resets it when something else replaces the form's value.
  • Before an autosave, the form compares that starting value with what is now stored. If someone else changed it, the editor does not discard anything. It applies the form's own edits on top of the stored section with the same three-way merge the sync uses (below), shows the merged section in the form, saves it, and shows an informational toast ("This block was updated while you were editing. Your changes were applied on top of the latest version."). It does not say who changed the block, because a publish synced in, a coding session on the same branch, another tab, and a discard all look the same from here.. The form only displays the merge while it is still on that section and has not emitted a newer value; otherwise its baseline stays what it displays, so the next save merges again.
  • For saved/global blocks, the save passes the version it built on as the expect base.
  • If the server still answers 409 (the client's cache was older than the branch), useSaveBlock re-reads the decofile and the editor retries once, merging against the fresh read. Only a second refusal falls back to reloading the form, with a neutral error toast. The rollback of the optimistic write is skipped on a conflict, so it can't put the stale copy back over the fresh read.

3. Sync conflicts merge blocks value by value (git-compat.ts, block-merge.ts in @decocms/shared, shared with the editor)

  • When the git merge conflicts, every .deco/blocks/*.json file modified on both sides gets a three-way JSON merge against the merge base:
    • Objects merge key by key.
    • The branch wins only values that both sides changed differently.
    • Arrays merge position by position only when neither side changed the length and no position changed on both sides. Otherwise the branch's whole array wins, so a reorder on one side and an edit on the other can't splice fields from two items together.
  • Files that are not blocks keep today's branch-wins behaviour, as do blocks added or removed on one side and files that aren't valid JSON. The squash message is unchanged.

Not in this PR

  • useMoveBlocks, useDeleteBlock, and the blog editor's patchDecofile calls don't send expect yet. They keep the old last-write-wins behaviour.
  • The automatic "get latest" fast-forward is unchanged. With the checks above, a stale form after a fast-forward now reloads instead of overwriting.

How did you verify your code works?

  • bun run verify (fmt, oxlint, knip, 10,910 unit tests) and bun run check (all workspaces' tsc) pass.
  • New unit tests:
    • fingerprint.test.ts: key order, JSON round trip, secret value ignored, plaintext secret refused.
    • block-merge.test.ts (shared): per-key merge, both-changed value, additions and deletions, array position merge, a reorder plus an edit keeping the branch's array, length change, key order.
    • form-base.test.ts: how the form's starting point is tracked and reset.
    • commit-coalescer.test.ts: accept/reject against head, null expectation, ordering inside one coalesced batch.
  • New and updated Playwright specs, run locally against the GitHub stub (decofile-api, fast-preview-git-sync, site-editor-stale-form, site-editor-breadcrumbs, cms-publish-surface): 30 passed.
    • decofile-api: a stale expect gets a 409 and leaves the branch head untouched. Redone against a fresh read, the edit lands on top. Covers null semantics and schema rejections.
    • fast-preview-git-sync: the conflict test now asserts that the base's edit to a block survives next to the branch's edit, and that the branch wins only the value both changed. It used to assert that the branch's whole file wins.
    • site-editor-stale-form (browser): for a saved block and for an inline section, open the form, change the block through the API as another editor would, then type in the form. The stored block ends up with both changes, the form shows the merged values (checked on the nested field the other editor changed), and a second edit keeps the other editor's change too.
    • site-editor-stale-form, the reported path end to end: the form is open on a draft, another draft publishes a change to the block, "get latest" fast-forwards this draft onto it, the editor re-reads the draft while the form keeps its old copy, and the user types. The editor merges before writing (asserted: no PATCH got a 409) and the draft stores both changes. On main's editor code the same steps store the block with the published change reverted, which is the bug as reported.
  • I checked that the new tests fail without the fix: the two new API assertions fail with the old coalescer and git-compat.ts, and both browser tests fail with the old use-save-block.ts and sections-editor.tsx.
  • Not run: the full e2e suite (CI will run it).

Checked against real project history

Before publishing, I checked how this would behave on three production projects with Fast Preview and heavy CMS use (roughly 1,400 draft PRs merged since September, 1.8k–2k blocks each):

  • Block data: none has invalid JSON blocks, block keys that decode to the same name, plaintext secrets, or blocks over 1 MB, which are the cases where a client and server fingerprint could disagree. Fingerprinting the largest block (~680 KB) takes about 2 ms.
  • Concurrent editors: in the last 30 days, only one draft branch had more than one editor's CMS chats, so 409s between two people on the same branch should be rare.
  • Syncs: about 700 draft syncs ran in that period, and 14 hit a git conflict and went branch-wins. I replayed those 14 with the new merge, estimating the merge base as the base's version of each file closest to the draft's:
    • In 5 of them, the old sync dropped the base's edits (up to 36 values in one block, in a block the draft had not really changed) and the new merge keeps them.
    • In the rest, the result is the same as before. Several page blocks fall back to the branch's whole sections array because one side changed the array's length.
    • None came out worse.
  • Not covered: one project lives in a nested package of a repository I can't read, so its history wasn't checked. The code paths handle a package path the same way.
  • Outside writer: two of the projects also get automated .deco syncs on the default branch from another system. Those are just more base-side changes, which is the case the value-level merge is for.

Screenshots/Demonstration

The form after another editor changed the saved block it was editing. The form's edit was applied on top of the stored block, and both changes were saved:

Section form after merging another editor's change

How to Test

  1. Open a Fast Preview project in two browsers as two users, each on its own draft.
  2. In browser A, open a global section (e.g. the header) in the Site Editor and leave the form open.
  3. In browser B, change a different field of the same section and publish.
  4. In browser A, click "Get latest" or wait for the automatic sync, then edit a field.
  5. Expected: A sees the "This block was updated while you were editing…" toast, the form shows B's value next to A's edit, and publishing A keeps both changes. Before this PR, A's save reverted B's change.

Migration Notes

None. The expect field is optional, so older clients keep the current behaviour. No flag: the check only runs for writes that ask for it.

Review Checklist

  • PR title is clear and descriptive
  • Changes are tested and working
  • Documentation is updated (if needed)
  • No breaking changes