fix: apply value-sync auto-resolutions to the block the engine receives

.changeset/apply-autoresolve-to-engine-block.md

@@ -0,0 +1,15 @@
+---
+'@portabletext/editor': patch
+---
+
+fix: apply value-sync auto-resolutions to the block the engine receives
+
+When an `update value` carried a block the editor could repair
+automatically (a child missing its `_key`, an empty `children` array,
+unused `markDefs`), the repair was emitted as patches while the raw,
+un-repaired block proceeded into the editor. The editor then held an
+invalid shape that diverged from the document: the emitted patch minted
+one key, internal normalization minted another. The repair is now
+applied to the block before it enters the editor, so the emitted patch
+and the editor state agree, and the editor never holds the un-repaired
+shape.

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

@@ -1,4 +1,4 @@
-import type {Patch} from '@portabletext/patches'
+import {applyAll, type Patch} from '@portabletext/patches'
 import {isSpan, isTextBlock, type PortableTextBlock} from '@portabletext/schema'
 import type {ActorRefFrom} from 'xstate'
 import {
@@ -703,7 +703,13 @@ function syncBlock({
     )
 
     if (validation.valid || validation.resolution?.autoResolve) {
-      const engineBlock = toEngineBlock(block, {
+      // Apply the auto-resolution to the block the engine receives, so the
+      // engine never holds the un-repaired shape.
+      const repairedBlock =
+        !validation.valid && validation.resolution?.autoResolve
+          ? (applyAll([block], validation.resolution.patches).at(0) ?? block)
+          : block
+      const engineBlock = toEngineBlock(repairedBlock, {
         schemaTypes: context.schema,
       })
 
@@ -785,8 +791,20 @@ function syncBlock({
   }
 
   if (validation.valid || validation.resolution?.autoResolve) {
+    // Apply the auto-resolution to the block the engine receives. Without
+    // this the repair only exists as the emitted patches above while the
+    // raw block proceeds into the engine: the engine ends up holding the
+    // un-repaired shape (e.g. a keyless child), diverging from the document
+    // that received the minted key, and the next sync against that invalid
+    // engine state can kill the sync.
+    const repairedBlock =
+      !validation.valid && validation.resolution?.autoResolve
+        ? (applyAll(validationValue, validation.resolution.patches).at(0) ??
+          block)
+        : block
+
     if (oldBlock._key === block._key && oldBlock._type === block._type) {
-      debug.syncValue('Updating block', oldBlock, block)
+      debug.syncValue('Updating block', oldBlock, repairedBlock)
 
       withoutNormalizing(editorEngine, () => {
         withRemoteChanges(editorEngine, () => {
@@ -795,22 +813,22 @@ function syncBlock({
               context,
               editorEngine,
               oldEngineBlock,
-              block,
+              block: repairedBlock,
               index,
             })
           })
         })
       })
     } else {
-      debug.syncValue('Replacing block', oldBlock, block)
+      debug.syncValue('Replacing block', oldBlock, repairedBlock)
 
       withoutNormalizing(editorEngine, () => {
         withRemoteChanges(editorEngine, () => {
           withoutPatching(editorEngine, () => {
             replaceBlock({
               context,
               editorEngine,
-              block,
+              block: repairedBlock,
               index,
             })
           })

packages/editor/tests/container-normalization.test.tsx

@@ -398,7 +398,7 @@ describe('container normalization', () => {
           children: [
             {
               _type: 'span',
-              _key: 'k4',
+              _key: 'k3',
               text: '',
               marks: [],
             },

packages/editor/tests/event.operation.test.tsx

@@ -105,17 +105,13 @@ describe('event.operation', () => {
     expect(patches).toEqual([])
   })
 
-  test('Scenario: Normalization fix operations are delivered adjacent to their trigger', async () => {
+  test('Scenario: Auto-resolved blocks arrive repaired in their own insert operation', async () => {
     const {editor} = await createTestEditor()
     const operations = collectOperations(editor)
 
-    // A text block with no children triggers a normalization fix that
-    // inserts a placeholder span. Value sync applies in a batch, so the fix
-    // runs at the batch close and is delivered after its trigger
-    // (application order). The opposite, nested order for unbatched applies
-    // is pinned at the unit level in `operation-channel.test.ts`; the
-    // public contract is only adjacency, which is why the docs steer
-    // consumers toward snapshot-seeded recompute.
+    // A text block with no children is auto-resolved by `validateValue` at
+    // sync ingress: the placeholder span is part of the inserted node
+    // itself, not a separate normalization fix operation.
     editor.send({type: 'update value', value: [emptyBlock('b1')]})
 
     await vi.waitFor(() => {
@@ -128,16 +124,61 @@ describe('event.operation', () => {
           type: 'insert',
           path: [0],
           position: 'before',
-          node: emptyBlock('b1'),
+          node: {
+            ...emptyBlock('b1'),
+            children: [{_type: 'span', _key: 'k2', text: '', marks: []}],
+          },
+        },
+      ])
+    })
+  })
+
+  test('Scenario: Normalization fix operations are delivered adjacent to their trigger', async () => {
+    const {editor} = await createTestEditor()
+    const operations = collectOperations(editor)
+
+    // Duplicate child keys pass value validation (no dup-key check at
+    // ingress) and are repaired by engine normalization, which renames the
+    // second occurrence. Value sync applies in a batch, so the fix runs at
+    // the batch close and is delivered after its trigger (application
+    // order). The opposite, nested order for unbatched applies is pinned
+    // at the unit level in `operation-channel.test.ts`; the public
+    // contract is only adjacency, which is why the docs steer consumers
+    // toward snapshot-seeded recompute.
+    // The differing marks keep the two spans from also triggering the
+    // adjacent same-mark merge, so the rename is the only fix.
+    const dupBlock: PortableTextBlock = {
+      _type: 'block',
+      _key: 'b1',
+      style: 'normal',
+      markDefs: [],
+      children: [
+        {_type: 'span', _key: 'dup', text: 'one', marks: []},
+        {_type: 'span', _key: 'dup', text: 'two', marks: ['strong']},
+      ],
+    }
+    editor.send({type: 'update value', value: [dupBlock]})
+
+    await vi.waitFor(() => {
+      expect(operations).toEqual([
+        {
+          type: 'unset',
+          path: [{_key: 'k0'}],
         },
         {
           type: 'insert',
-          path: [{_key: 'b1'}, 'children', 0],
+          path: [0],
           position: 'before',
-          node: {_type: 'span', _key: 'k3', text: '', marks: []},
+          node: dupBlock,
+        },
+        {
+          type: 'set',
+          path: [{_key: 'b1'}, 'children', 1, '_key'],
+          value: 'k2',
           inverse: {
-            type: 'unset',
-            path: [{_key: 'b1'}, 'children', {_key: 'k3'}],
+            type: 'set',
+            path: [{_key: 'b1'}, 'children', 1, '_key'],
+            value: 'dup',
           },
         },
       ])

packages/editor/tests/event.update-value.test.tsx

@@ -1,6 +1,11 @@
 import {createTestKeyGenerator} from '@portabletext/test'
 import {describe, expect, test, vi} from 'vitest'
-import {defineSchema, type EditorEmittedEvent} from '../src'
+import {
+  defineSchema,
+  type EditorEmittedEvent,
+  type MutationEvent,
+  type Patch,
+} from '../src'
 import {EventListenerPlugin} from '../src/plugins/plugin.event-listener'
 import {createTestEditor} from '../src/test/vitest'
 import {toTextspec} from '../test-utils/to-textspec'
@@ -1912,3 +1917,156 @@ describe('event.update value: adjacent same-mark spans', () => {
     })
   })
 })
+
+describe('event.update value: auto-resolved invalid blocks', () => {
+  // Regression: `validateValue` auto-resolutions (e.g. minting a missing
+  // child `_key`) were emitted as outbound patches while the *raw* block
+  // proceeded into the engine. The engine ended up holding the un-repaired
+  // shape (a keyless child), diverging from the document that received the
+  // minted key, and the next sync against that invalid engine state killed
+  // the sync silently.
+  const keylessChildBlock = {
+    _key: 'b0',
+    _type: 'block',
+    children: [{_type: 'span', text: 'hello changed', marks: []}],
+    markDefs: [],
+    style: 'normal',
+  }
+
+  test('Scenario: a mid-session update with a keyless child is repaired once and the sync survives', async () => {
+    const patches: Array<Patch> = []
+    const {editor} = await createTestEditor({
+      keyGenerator: createTestKeyGenerator(),
+      schemaDefinition: defineSchema({}),
+      initialValue: [
+        {
+          _key: 'b0',
+          _type: 'block',
+          children: [{_key: 's0', _type: 'span', text: 'hello', marks: []}],
+          markDefs: [],
+          style: 'normal',
+        },
+      ],
+      children: (
+        <EventListenerPlugin
+          on={(event) => {
+            if (event.type === 'patch') {
+              patches.push(event.patch)
+            }
+          }}
+        />
+      ),
+    })
+
+    // A changed block arrives whose span lost its `_key`.
+    editor.send({type: 'update value', value: [keylessChildBlock]})
+
+    // The auto-resolution is emitted as a patch AND applied to the block
+    // the engine receives: one key, minted once, on both sides.
+    await vi.waitFor(() => {
+      expect(patches).toEqual([
+        {
+          type: 'set',
+          path: [{_key: 'b0'}, 'children', 0],
+          value: {
+            _type: 'span',
+            _key: 'k2',
+            text: 'hello changed',
+            marks: [],
+          },
+        },
+      ])
+      expect(editor.getSnapshot().context.value).toEqual([
+        {
+          _key: 'b0',
+          _type: 'block',
+          children: [
+            {_key: 'k2', _type: 'span', text: 'hello changed', marks: []},
+          ],
+          markDefs: [],
+          style: 'normal',
+        },
+      ])
+    })
+
+    // The sync is still alive: a later update still lands.
+    editor.send({
+      type: 'update value',
+      value: [
+        {
+          _key: 'b0',
+          _type: 'block',
+          children: [{_key: 's9', _type: 'span', text: 'recovered', marks: []}],
+          markDefs: [],
+          style: 'normal',
+        },
+      ],
+    })
+
+    await vi.waitFor(
+      () => {
+        expect(editor.getSnapshot().context.value).toEqual([
+          {
+            _key: 'b0',
+            _type: 'block',
+            children: [
+              {_key: 's9', _type: 'span', text: 'recovered', marks: []},
+            ],
+            markDefs: [],
+            style: 'normal',
+          },
+        ])
+      },
+      // The sync machine parks in `busy` while its own emitted mutation
+      // flushes and re-checks on a 1s timer, so the recovery value can
+      // take a beat over a second to land.
+      {timeout: 5000},
+    )
+  })
+
+  test('Scenario: a startup value with a keyless child is repaired in the engine without emitting patches', async () => {
+    const patches: Array<Patch> = []
+    const mutations: Array<MutationEvent> = []
+    const {editor} = await createTestEditor({
+      keyGenerator: createTestKeyGenerator(),
+      schemaDefinition: defineSchema({}),
+      initialValue: [
+        {
+          _key: 'b0',
+          _type: 'block',
+          children: [{_type: 'span', text: 'hello', marks: []}],
+          markDefs: [],
+          style: 'normal',
+        },
+      ],
+      children: (
+        <EventListenerPlugin
+          on={(event) => {
+            if (event.type === 'patch') {
+              patches.push(event.patch)
+            }
+            if (event.type === 'mutation') {
+              mutations.push(event)
+            }
+          }}
+        />
+      ),
+    })
+
+    await vi.waitFor(() => {
+      expect(editor.getSnapshot().context.value).toEqual([
+        {
+          _key: 'b0',
+          _type: 'block',
+          children: [{_key: 'k2', _type: 'span', text: 'hello', marks: []}],
+          markDefs: [],
+          style: 'normal',
+        },
+      ])
+    })
+
+    // No mutation leaves a pristine editor on open.
+    expect(patches).toEqual([])
+    expect(mutations).toEqual([])
+  })
+})

packages/editor/tests/pteWarningsSelfSolving.test.tsx

@@ -54,7 +54,7 @@ describe('when PTE would display warnings, instead it self solves', () => {
             _type: 'block',
             children: [
               {
-                _key: 'k3',
+                _key: 'k2',
                 _type: 'span',
                 text: 'Hello with a new key',
                 marks: [],
@@ -174,7 +174,7 @@ describe('when PTE would display warnings, instead it self solves', () => {
             _type: 'block',
             children: [
               {
-                _key: 'k3',
+                _key: 'k2',
                 _type: 'span',
                 text: '',
                 marks: [],
@@ -188,7 +188,7 @@ describe('when PTE would display warnings, instead it self solves', () => {
             _type: 'block',
             children: [
               {
-                _key: 'k5',
+                _key: 'k3',
                 _type: 'span',
                 text: '',
                 marks: [],