RTC: Add separate doc persistence endpoint

[alecgeatches]

What?

Closes #78867.

Add a new wp-sync/v1/save endpoint for persisting a CRDT doc outside of a regular post save.

Why?

@jsnajdr called out two undesirable behaviors with the current CRDT persistence setup in #78799:

1. When entities are loaded, we may determine the server-side CRDT document is out of sync with editor content during getEntityRecord(), and then trigger a subsequent saveEntityRecord() in order to update the CRDT post meta. This get operation side effect is unexpected, and has additional side effects both in the editor and on the server:

   saveEntityRecord, the REST endpoint, and the server-side PHP wp_update_post have the semantics that the user really saved a new version of the post, and modified it in some meaningful way. But saving the CRDT document is just a RTC bookkeeping, the user is not really saving anything. It's triggered even by purely read-only actions.

   In wp-data, saveEntityRecord makes the UI think that the post is really saving, and it disables the "Publish" button for a moment, for example.

   wp_update_post calls many popular filters and actions that plugins hook into, and the assumption is that we're saving a post.

   It also changes a post's modified date, which can be incorrect when post content was not saved and only the CRDT meta was updated. We just want to update a persisted CRDT doc, not all of these other actions to occur.

2. We send no-op { id } save for non-persisted entities like categories due to a bug:

   It seems there is a case where the payload is too small and doesn't even include the CRDT doc. Sometimes when creating a new post I see a request to update a category, where the payload includes only the id:

   POST /wp/v2/categories/1 { id: 1 }
   

   This is triggered by a getEntityRecord( 'taxonomy', 'category' ) read.

How?

When we're just saving CRDT document snapshots, use the new dedicated wp-sync/v1/save REST endpoint and not the saveEntityRecord save flow. Directly updating meta avoids wp_update_post(), post-save hooks, editor save UI state, and post_modified changes for bookkeeping-only CRDT repairs.

The sync config has a new supportsPersistence flag so we don't need to guess at which entities should receive an update. Unsupported entity rooms such as categories are rejected for CRDT doc persistence and also skipped client side.

On the client side, saveCRDTDoc() serializes the current Yjs document via the sync manager and posts it to /wp-sync/v1/save. Saves are queued per room so a slower, older serialized snapshot cannot complete after a newer one and overwrite it.

An important note: User-initiated post saves (e.g. clicking the "Save" button) still include the CRDT doc in the normal post save payload as before. This is explained in the existing prePersistPostType() call:

gutenberg/packages/core-data/src/entities.js

Lines 309 to 313 in cb9c492

	// Add meta for the persisted CRDT document during real post saves so the
	// saved post and CRDT snapshot are committed in the same request. We don't
	// want a post save to fail but a CRDT update to succeed or vice versa.
	// CRDT repair uses /wp-sync/v1/save to avoid post-save side effects.
	if ( persistedRecord ) {

Change to waitForConvergence() in E2E test

A small change in behavior worth calling out. Before, on every CRDT save (user-facing or side-effect caused), every user would re-fetch the _crdt_document meta locally. This no longer happens when a side-effect CRDT repair hits /wp-sync/v1/save.

The e2e utility function waitForConvergence() was added as part of fuzzer testing in #77666, and previously had an includeCrdtDocument key that was causing a test failure in collaboration-title-reload.spec.ts. The test failure was specifically due to the includeCrdtDocument: true flag only used in this test, and was failing because the page was checking for stale record?.meta?._crdt_document convergence.

This was removed. Previously this worked because a CRDT doc repair generated a "Save" event, which caused all other peers to re-fetch the document from the server. I briefly tried a separate persistedAt flag to make all other users refetch in the background, but after some investigation it's unnecessary.

After the background CRDT repair persists through /wp-sync/v1/save, the server-side document is correct and users have no need to refetch. The _crdt_document meta is otherwise unused in an already active session. The new CRDT doc is still available when a post is loaded from scratch, and explicit post saves still persist the CRDT doc through normal post meta. But once a browser has an active Yjs document, the in-browser source of truth is the Yjs document and editor state, not the local entity record's _crdt_document field. So we don't need to force-refresh _crdt_document as we did before.

Testing Instructions

General testing instructions. This will affect a few processes, so some all-around RTC testing would be helpful.

1. Enable real-time collaboration in Settings -> Writing.
2. Open a post in the editor with network tools enabled.
3. Make an edit and save. Confirm the post save still succeeds, sending a POST to wp/v2/posts/<post_id>.
4. Open the same post in another browser session and confirm RTC edits still sync.

CRDT repairs

1. To test a CRDT repair, keep the post open in two browsers and use WPCLI to append some content to the post. Below, replace the ID in $post_id = <ID>; with the actual post ID:

   // Replace <ID> with the post ID
   npm run wp-env run cli wp eval '$post_id = <ID>; $p = get_post($post_id); wp_update_post(["ID" => $post_id, "post_content" => $p->post_content . "\n\n<!-- wp:paragraph -->\n<p>Updated content</p>\n<!-- /wp:paragraph -->"]);'

2. Refresh one of the browsers. After loading, you should see a wp-sync/v1/save request and NOT a request to wp/v2/posts/<post_id>. The modified content from DB should be loaded onto the page. The new content should sync between both browsers:

   repair-sync.mov

3. Refresh the other browser and ensure that the "Updated content" block is still included in post content.

Automated coverage

npm run test:unit -- packages/core-data/src/utils/test/save-crdt-doc.js packages/core-data/src/test/resolvers.js packages/sync/src/test/manager.ts
npm run test:unit:php:base -- phpunit/tests/collaboration/wpSyncSaveServer.php phpunit/tests/collaboration/wpSyncConfig.php phpunit/tests/collaboration/restAutosavesController.php

Use of AI Tools

AI assistance: Yes
Tool(s): Codex
Used for: The whole process.

[github-actions]

Size Change: +171 B (0%)

Total Size: 8.44 MB

📦 View Changed

Filename	Size	Change
build/scripts/block-editor/index.min.js	380 kB	+4 B (0%)
build/scripts/core-data/index.min.js	31.7 kB	+167 B (+0.53%)

_{compressed-size-action}

[jsnajdr]

update_post_meta internally does the check whether old value is equal to the new value, we don't need to do it separately.

The return value is false in case of equality. That also means that the false return value can't be treated as a strict error. An is_wp_error check on the return value is probably better.

[alecgeatches]

I didn't know about that behavior without the $prev_value flag! However, it looks like update_post_meta() and the internal update_metadata() function both return int|bool and not a WP_Error. Overall this makes it a bit confusing, as $updated will be false if the document fails to save, but also if the document is the same as already saved. A WP_Error would be much better.

I pushed a change in 3aa0658 that avoids the get_post_meta() call unless we receive false first and then need to differentiate whether the meta change represents an error or a same-value change.

[jsnajdr]

This check is already done in permission_callback, handle_request should only do the business logic.

[alecgeatches]

Good catch, fixed in ee8bfeb.

[jsnajdr]

There is a native Promise.withResolvers helper for this.

[alecgeatches]

I fixed this in b2095bf, but ended up reverting it back in cb9c492. I was seeing test failures related to withResolvers():

https://github.com/WordPress/gutenberg/actions/runs/26975840218/job/79602551098?pr=78891

Error: TypeError: Promise.withResolvers is not a function

This passes locally, but it appears we only run CI JS unit tests on node 20, which doesn't have withResolvers().

[jsnajdr]

Oh, there is Node.js 20 🙁 Let's do it some other day then, when we upgrade Node.

[jsnajdr]

await new Promise( r => setTimeout( 0, r ) ) is a more reliable way to do this flush. It will resolve only after all microtasks have reliably run, no matter what is the count.

[alecgeatches]

Fixed this and the above withResolvers note in b2095bf, thanks!

[jsnajdr]

The MAX_BODY_SIZE is already declared in the args option of register_rest_route. The default validate_callback implementation should therefore do the check, it doesn't need to be implemented separately.

[github-actions]

Flaky tests detected in e481c7e.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/27150061323
📝 Reported issues:

• [Flaky Test] Text blocks sync modifications between users #78855 in /test/e2e/specs/editor/collaboration/collaboration-block-gauntlet.spec.ts

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: alecgeatches <alecgeatches@git.wordpress.org>
Co-authored-by: jsnajdr <jsnajdr@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[alecgeatches]

@danluu Just wanted to note a test change made in this PR from the fuzzer-added tests in #77666, see "Change to waitForConvergence() in E2E test" in the PR description.

In short, in addition to checking the live blocks for convergence, we were checking a local _crdt_document meta value. This PR removes CRDT repairs from the normal post save path, so we don't otherwise require all users to refetch that value as before. I don't think checking that local meta adds any value, as we already update the server copy and bundle the CRDT doc from live state on save. Fixing it requires re-adding the "everybody refetches" logic that is unnecessary otherwise.

[alecgeatches]

@jsnajdr Thank you for the review notes above! I've addressed them and finalized the PR and description for review. I'll also want to get a couple of other RTC folks in here to do general testing as well.

[jsnajdr]

Let's ship This will fix many existing issues.

[jsnajdr]

Oh, there is Node.js 20 🙁 Let's do it some other day then, when we upgrade Node.

[jsnajdr]

We don't really use the response, do we? It could be left empty.

Also, wrapping in new WP_REST_Response and 200 status is probably redundant: that's the default behavior. In the default case we only need to return the response array.

[jsnajdr]

Looking at usages of update_post_meta in the WordPress codebase, the return value is mostly not checked at all 🤷‍♂️ It's probably quite hard for the update to fail. 🙂

[alecgeatches]

Okay, good to know. I left in the defensive check for now, and ideally we rarely or never hit that code path anyway.

[alecgeatches]

@jsnajdr Addressed the WP_REST_Response comment above and merged!