RTC: Preserve unsaved blocks when reconciling a persisted CRDT doc

[giteshsarvaiya]

Fixes #80004

What

_applyPersistedCrdtDoc in packages/sync/src/manager.ts compares the
persisted CRDT doc against the real entity record to find invalidated keys,
then builds a changes object mapping each invalidated key to
record[key].

When 'blocks' is an invalidated key, record['blocks'] is always
undefined — REST post records never expose a blocks property, only
content (serialized HTML). That undefined reached
applyPostChangesToCRDTDoc's 'blocks' case in
packages/core-data/src/utils/crdt.ts without an accompanying content
value, so it took the "wipe blocks" branch instead of the existing
mergeContentWithoutBlocks merge path — losing stable block identities and,
on repeated reconciliation (e.g. viewing a server-side autosave), stacking
duplicate content (hello 123 → hello 123 123 → ...).

The fix: when 'blocks' is invalidated, also carry record.content along
in the changes payload, so the existing (already correct)
mergeContentWithoutBlocks path in crdt.ts gets what it needs to
reconcile blocks from content instead of wiping them. No changes needed in
crdt.ts.

Repro

1. Enable Real-Time Collaboration.
2. Open a published post with a paragraph that says hello.
3. Type 123 so it reads hello 123. Do not click Save.
4. Wait ~1 minute for WordPress's server-side autosave to fire.
5. Leave the editor without saving, then reopen the post. A "there is a more
   recent autosave" notice appears.
6. Click "View the autosave".

Before: hello 123 becomes hello 123 123 (repeating the cycle keeps
stacking duplicates).
After: stays hello 123.

Video Before

80004.issue.mp4

Video After

80004.pr.mp4

Testing

• Added a regression test in packages/sync/src/test/manager.ts asserting
  applyChangesToCRDTDoc is called with { blocks: undefined, content: mockRecord.content } when blocks is invalidated (previously it would
  only receive { blocks: undefined }).
• npm run test:unit -- packages/sync: 224/224 passing.
• npm run lint:js / npm run format on changed files: clean.

Design note (open question for reviewers)

packages/sync is otherwise a fully entity-agnostic package — nothing else
in manager.ts references WordPress-post-specific field names, and the same
manager code is reused as-is for non-post entities (e.g. taxonomies via
defaultSyncConfig). This fix introduces the first WP-post-specific
special case (key === 'blocks' && record.content) directly into that
generic file, duplicating knowledge that already lives correctly in the
post-specific applyPostChangesToCRDTDoc/mergeContentWithoutBlocks in
packages/core-data/src/utils/crdt.ts.

Possible future enhancement

A cleaner long-term shape would be a new optional SyncConfig hook that lets
each entity-specific config own this kind of "field A needs field B to
reconstruct" logic, keeping manager.ts itself free of entity-specific
literals — consistent with how applyChangesToCRDTDoc/
getChangesFromCRDTDoc already delegate entity-specific behavior today.

// packages/sync/src/types.ts
export interface SyncConfig {
    applyChangesToCRDTDoc: ( ydoc: Y.Doc, changes: Partial< ObjectData > ) => void;
    getChangesFromCRDTDoc: ( ydoc: Y.Doc, editedRecord: ObjectData ) => ObjectData;
    // New: build the `changes` payload for a set of invalidated keys. Lets an
    // entity-specific config express "key X needs record[Y] to reconcile
    // correctly" without the generic manager knowing about entity field names.
    getChangesForInvalidatedKeys?: (
        invalidatedKeys: string[],
        record: ObjectData
    ) => Partial< ObjectData >;
    // ...existing hooks
}

// packages/sync/src/manager.ts — _applyPersistedCrdtDoc
const changes = syncConfig.getChangesForInvalidatedKeys
    ? syncConfig.getChangesForInvalidatedKeys( invalidatedKeys, record )
    : invalidatedKeys.reduce< Partial< ObjectData > >( ( acc, key ) => {
          acc[ key ] = record[ key ];
          return acc;
      }, {} );

// packages/core-data/src/entities.js — post syncConfig
getChangesForInvalidatedKeys( invalidatedKeys, record ) {
    const changes = invalidatedKeys.reduce( ( acc, key ) => {
        acc[ key ] = record[ key ];
        return acc;
    }, {} );
    if ( invalidatedKeys.includes( 'blocks' ) && record.content ) {
        changes.content = record.content;
    }
    return changes;
}

Not proposing this be built now — it's a bigger surface change (new
interface, new implementation in core-data, new tests) for what's
currently a single call site. Sketching it here so the tradeoff is visible
and the follow-up is easy to pick up if a second entity type ever needs
similar treatment.

Precedent: this is how the codebase already handles this class of fix

Checked git history for how similar blocks/content coupling issues were
handled previously in this exact code — the established pattern is narrow,
inline fixes, not new abstractions:

• Real-time collaboration: Apply only detected changes from the persisted CRDT document #74668 (50b0a31ec0, "Apply only detected changes from the persisted
  CRDT document") introduced the very invalidatedKeys.reduce(...) line this
  PR touches. It shipped as a fully generic, naive per-key copy from day
  one — no entity-specific hook.
• Real-time collaboration: Sync post content and undefined blocks value #75437 (001a256148, "Sync post content and undefined blocks
  value") introduced the blocks-needs-content reconciliation logic this
  PR's fix relies on (mergeContentWithoutBlocks). It landed as an inline
  case 'blocks' special case directly in crdt.ts, not through any
  generic mechanism.
• RTC: Sync the content even if its a function #76796 (1263f906e3, "Sync the content even if its a function")
  landed another narrow, inline fix straight into crdt.ts's existing
  switch statement, again with no new abstraction.

Given that track record, an inline special case here is consistent with how
this part of the codebase has always evolved — generalize later, if/when a
second real case shows up.

Question for reviewers: should the getChangesForInvalidatedKeys hook
sketched above be added now, deferred to a follow-up PR, or skipped
entirely given the precedent above?

Related / not in scope

• RTC can cause an unsaved change to persist and cause data loss from the original post #76402's core bug (unsaved changes persisting after leaving without
  saving) is a separate, larger root cause — not addressed here.
• PR RTC: Preserve collaborators' unsaved edits when another user saves #79936 (preserving collaborators' unsaved edits on save) is a related
  but distinct save-time bug in packages/core-data/src/actions.js — no
  overlap with this fix.

---

This PR was developed with AI assistance (Claude Code, opus 4.8) for
investigation, implementation, and testing. All changes were reviewed and
verified by me before submission.

[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: giteshsarvaiya <giteshsarvaiya@git.wordpress.org>
Co-authored-by: alecgeatches <alecgeatches@git.wordpress.org>

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

[github-actions]

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @giteshsarvaiya! In case you missed it, we'd love to have you join us in our Slack community.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

[alecgeatches]

@giteshsarvaiya I'm able to reproduce this problem, and it's a good find, but I'm not sure if the fix here works. In my testing, content duplication seems to happen only when another user is present in the post (to hold the original 123) addition. Without another user, the problem doesn't reproduce. This is true both on trunk and in this PR branch.

Here's the reproduction steps on trunk with one user:

autosave-trunk.mov

Note that I've set "config": { "AUTOSAVE_INTERVAL": 20 } in .wp-env.json to reduce the autosave time for the video, but it works the same with a 60s autosave.

On trunk, if we have just one user, I can't reproduce the bug. You'll notice when the post reloads the 123 text isn't included in the CRDT doc, because it was not persisted and there is no other user holding that live CRDT change. Then the restore works just fine as expected and doesn't duplicate content.

If another user is present in the document, I can reproduce content duplication, but the changes in this PR don't appear to fix it. Below, I repeated the same steps on the current PR branch fix/80004-rtc-crdt-blocks-reconciliation, freshly built, with another user present in the post:

autosave-pr-branch.mov

This seems to match your before and after videos. In the "before" video, after reloading, the 123 text re-appears which sort of looks like an active session is syncing that content, but in the "after" video the 123 text doesn't appear on reload. I was only able to reproduce this by having additional users in my reproduction.

In short, trunk and this PR branch don't reproduce the issue with a single user, but both reproduce the issue with a collaborator present. The problem here seems real, but I'm not sure the fix is working. Am I missing something? Thanks!

[giteshsarvaiya]

Hi @alecgeatches

Really appreciate you taking the time to dig into this and record those videos. Your finding about the collaborator-present case was a really useful lead and pushed me to test this a lot more rigorously than I had before raising the PR.

I ran the repro roughly 20-30 times today, varying a few conditions (with/without another collaborator, immediate vs. delayed navigation back into the post, hard reload vs. in-app navigation). It reproduced about 7-10 of those times, so it's real, but noticeably flaky rather than deterministic, and I was able to get it solo (no second person) a few times too, not just with a collaborator present.

I don't yet have a confirmed root cause for what makes it reproduce on some attempts and not others. I have a few leads (something related to timing around navigation/reconnection seems to matter) but nothing solid enough to state as the actual mechanism yet.

Given that, I don't think this PR is ready to merge as-is. The fix I proposed may not be addressing the real trigger. Going to keep digging and will follow up here once I have a clearer picture. Thanks again for the thorough testing and for sharing your repro, genuinely helpful in narrowing this down.

P.S. I am afk on 16th and 17th (IST).
Will be back after that and contribute further on issues and PRs.

[alecgeatches]

Thank you for digging into it, @giteshsarvaiya! I'm not sure why I wasn't able to reproduce without a collaborator. If you need to do any weird steps like adding a sleep somewhere or a breakpoint to reproduce consistently, that's totally fine. There are a lot of possibilities for race conditions in RTC. Any fairly-consistent reproduction, even if it needs to change some conditions to make it consistent, it totally fine. Let me know if you find one!

[giteshsarvaiya]

Thank you for digging into it, @giteshsarvaiya! I'm not sure why I wasn't able to reproduce without a collaborator. If you need to do any weird steps like adding a sleep somewhere or a breakpoint to reproduce consistently, that's totally fine. There are a lot of possibilities for race conditions in RTC. Any fairly-consistent reproduction, even if it needs to change some conditions to make it consistent, it totally fine. Let me know if you find one!

This is in my priority list right now, will check out the possibilities, and keep the thread updated.