fix: keep normalization fallout of remote changes local (normalization echo corruption)

packages/editor/src/editor/remote-patches.ts

@@ -37,8 +37,25 @@ export function setupRemotePatches({
     let changed = false
 
     withRemoteChanges(editor, () => {
-      withoutNormalizing(editor, () => {
-        withoutPatching(editor, () => {
+      // `withoutPatching` must wrap `withoutNormalizing`, not the other way
+      // around: `withoutNormalizing` runs a final normalize pass on exit,
+      // and with the previous nesting that pass ran after patching was
+      // restored. Normalization fallout of remote application (e.g. merging
+      // adjacent same-mark spans a collaborator's formatting toggle left
+      // behind) was then emitted as local patches and pushed back to the
+      // server. Every receiving client pushed its own competing "cleanup"
+      // of the same structure, and the interleaved merges corrupted the
+      // shared document (fragments deleted after their text had moved,
+      // text applied twice). Normalization fallout of remote changes must
+      // stay local; the originating client runs the same normalization as
+      // a genuine local edit and pushes it, so the server still converges
+      // to the normalized form.
+      //
+      // NOTE (draft): blanket suppression conflicts with the self-solving
+      // documents contract; see the PR description for the enumeration and
+      // the proposed hold-and-discard alternative.
+      withoutPatching(editor, () => {
+        withoutNormalizing(editor, () => {
           pluginWithoutHistory(editor, () => {
             for (const patch of patches) {
               try {
@@ -59,9 +76,11 @@ export function setupRemotePatches({
             }
           })
         })
+        if (changed) {
+          normalize(editor)
+        }
       })
       if (changed) {
-        normalize(editor)
         editor.onChange()
       }
     })

packages/editor/src/editor/sync-machine.ts

@@ -600,10 +600,10 @@ function clearEditor({
   editorEngine: PortableTextEditorEngine
   doneSyncing: boolean
 }) {
-  withoutNormalizing(editorEngine, () => {
-    pluginWithoutHistory(editorEngine, () => {
-      withRemoteChanges(editorEngine, () => {
-        withoutPatching(editorEngine, () => {
+  withoutPatching(editorEngine, () => {
+    withoutNormalizing(editorEngine, () => {
+      pluginWithoutHistory(editorEngine, () => {
+        withRemoteChanges(editorEngine, () => {
           if (doneSyncing) {
             return
           }
@@ -640,9 +640,9 @@ function removeExtraBlocks({
 }) {
   let isChanged = false
 
-  withoutNormalizing(editorEngine, () => {
-    withRemoteChanges(editorEngine, () => {
-      withoutPatching(editorEngine, () => {
+  withoutPatching(editorEngine, () => {
+    withoutNormalizing(editorEngine, () => {
+      withRemoteChanges(editorEngine, () => {
         const childrenLength = editorEngine.snapshot.context.value.length
 
         if (value.length < childrenLength) {
@@ -707,9 +707,9 @@ function syncBlock({
         schemaTypes: context.schema,
       })
 
-      withoutNormalizing(editorEngine, () => {
-        withRemoteChanges(editorEngine, () => {
-          withoutPatching(editorEngine, () => {
+      withoutPatching(editorEngine, () => {
+        withoutNormalizing(editorEngine, () => {
+          withRemoteChanges(editorEngine, () => {
             editorEngine.apply({
               type: 'insert',
               path: [editorEngine.snapshot.context.value.length],
@@ -788,9 +788,9 @@ function syncBlock({
     if (oldBlock._key === block._key && oldBlock._type === block._type) {
       debug.syncValue('Updating block', oldBlock, block)
 
-      withoutNormalizing(editorEngine, () => {
-        withRemoteChanges(editorEngine, () => {
-          withoutPatching(editorEngine, () => {
+      withoutPatching(editorEngine, () => {
+        withoutNormalizing(editorEngine, () => {
+          withRemoteChanges(editorEngine, () => {
             updateBlock({
               context,
               editorEngine,
@@ -804,9 +804,9 @@ function syncBlock({
     } else {
       debug.syncValue('Replacing block', oldBlock, block)
 
-      withoutNormalizing(editorEngine, () => {
-        withRemoteChanges(editorEngine, () => {
-          withoutPatching(editorEngine, () => {
+      withoutPatching(editorEngine, () => {
+        withoutNormalizing(editorEngine, () => {
+          withRemoteChanges(editorEngine, () => {
             replaceBlock({
               context,
               editorEngine,

packages/editor/tests/remote-patches-normalization.test.tsx

@@ -0,0 +1,134 @@
+import type {Patch} from '@portabletext/patches'
+import {insert} from '@portabletext/patches'
+import {createTestKeyGenerator} from '@portabletext/test'
+import {describe, expect, test, vi} from 'vitest'
+import {createTestEditor} from '../src/test/vitest'
+
+/**
+ * Normalization fallout of remote patch application must stay local.
+ *
+ * When a collaborator's formatting toggle splits a span, the receiving
+ * editor's normalizer merges the resulting adjacent same-mark spans back
+ * together, which is correct local hygiene. But if those merge operations
+ * are emitted as local patches, every receiving client pushes its own
+ * competing "cleanup" of the same structure back to the server, and the
+ * interleaved merges corrupt the shared document (fragments unset after
+ * their text moved elsewhere, text applied twice). Field signature:
+ * formatting appears to work, then the formatted region's text is
+ * duplicated at the end of the block and the marks are gone.
+ */
+describe('remote patch application does not emit normalization patches', () => {
+  test('merging adjacent same-mark spans created by remote patches stays local', async () => {
+    const keyGenerator = createTestKeyGenerator()
+    const b1 = keyGenerator()
+    const s1 = keyGenerator()
+
+    const {editor} = await createTestEditor({
+      keyGenerator,
+      initialValue: [
+        {
+          _type: 'block',
+          _key: b1,
+          children: [{_type: 'span', _key: s1, text: 'foo', marks: []}],
+          markDefs: [],
+          style: 'normal',
+        },
+      ],
+    })
+
+    const emittedPatches: Patch[] = []
+    const patchSubscription = editor.on('patch', (event) => {
+      if ('patch' in event) {
+        emittedPatches.push(event.patch)
+      }
+    })
+    const emittedMutations: Patch[][] = []
+    const mutationSubscription = editor.on('mutation', (event) => {
+      emittedMutations.push(event.patches)
+    })
+
+    // A collaborator's split arrives: a second span with identical (empty)
+    // marks lands next to the existing one. The local normalizer merges the
+    // two spans into one.
+    editor.send({
+      type: 'patches',
+      patches: [
+        insert(
+          [{_type: 'span', _key: keyGenerator(), text: 'bar', marks: []}],
+          'after',
+          [{_key: b1}, 'children', {_key: s1}],
+        ),
+      ],
+      snapshot: undefined,
+    })
+
+    await vi.waitFor(() => {
+      const block = editor.getSnapshot().context.value[0] as {
+        children: {text: string}[]
+      }
+      // normalized: the spans merged into one
+      expect(block.children).toHaveLength(1)
+      expect(block.children[0]!.text).toBe('foobar')
+    })
+
+    // The merge itself must not be broadcast: no local patches, no mutation.
+    await new Promise((resolve) => setTimeout(resolve, 600))
+    expect(emittedPatches).toEqual([])
+    expect(emittedMutations).toEqual([])
+
+    patchSubscription.unsubscribe()
+    mutationSubscription.unsubscribe()
+  })
+
+  test('genuine local edits still emit patches after remote application', async () => {
+    const keyGenerator = createTestKeyGenerator()
+    const b1 = keyGenerator()
+    const s1 = keyGenerator()
+
+    const {editor} = await createTestEditor({
+      keyGenerator,
+      initialValue: [
+        {
+          _type: 'block',
+          _key: b1,
+          children: [{_type: 'span', _key: s1, text: 'foo', marks: []}],
+          markDefs: [],
+          style: 'normal',
+        },
+      ],
+    })
+
+    editor.send({
+      type: 'patches',
+      patches: [
+        insert(
+          [{_type: 'span', _key: keyGenerator(), text: 'bar', marks: []}],
+          'after',
+          [{_key: b1}, 'children', {_key: s1}],
+        ),
+      ],
+      snapshot: undefined,
+    })
+    await vi.waitFor(() => {
+      const block = editor.getSnapshot().context.value[0] as {
+        children: {text: string}[]
+      }
+      expect(block.children[0]!.text).toBe('foobar')
+    })
+
+    const emittedPatches: Patch[] = []
+    const subscription = editor.on('patch', (event) => {
+      if ('patch' in event) {
+        emittedPatches.push(event.patch)
+      }
+    })
+
+    editor.send({type: 'insert.text', text: '!'})
+
+    await vi.waitFor(() => {
+      expect(emittedPatches.length).toBeGreaterThan(0)
+    })
+
+    subscription.unsubscribe()
+  })
+})