From c7533ef30434b2ac50e70f57d0223f3abde3b9b1 Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Tue, 5 May 2026 10:36:06 -0700 Subject: [PATCH 1/5] Suggestions: Capture and apply block-insert-after suggestions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Inserts in Suggest mode now flow through the apply-and-tag pipeline: the block stays at its inserted position, the metadata.suggestion = pending-insert marker drives the visual treatment (UI ships in a follow-up), and auto-save persists a block-insert-after operation with anchor + parent context. Apply just clears the marker (the block is already in the tree); Reject runs removeBlock to undo. The interceptor's "new block" branch now captures the previous-tick parent + sibling order to compute anchorClientId (null = first child) and parentClientId (null = root). Descendants of another new block — a Group with nested children fires multiple new-block entries in one tick — are filtered out so only the top-level insertion is tagged; the descendants ride along inside the captured block snapshot. Refs #77434. --- .../components/suggestion-mode/provider.js | 59 +++++++++--- .../suggestion-mode/store-interceptor.js | 69 +++++++++++++- .../suggestion-mode/test/store-interceptor.js | 95 +++++++++++++++++++ 3 files changed, 204 insertions(+), 19 deletions(-) diff --git a/packages/editor/src/components/suggestion-mode/provider.js b/packages/editor/src/components/suggestion-mode/provider.js index 1825a99f239894..a6ac34997b7cd2 100644 --- a/packages/editor/src/components/suggestion-mode/provider.js +++ b/packages/editor/src/components/suggestion-mode/provider.js @@ -627,12 +627,12 @@ export function useSuggestionsProvider() { return; } - // Structural ops (block-remove for now, block-insert-after and - // block-move in follow-up PRs) can't ride the - // updateBlockAttributes path: their apply mutates the tree - // rather than a single block's attributes. Branch out, run the - // matching block-editor action, and short-circuit before the - // attribute-set rollback machinery below. + // Structural ops (block-remove, block-insert-after; block-move + // ships in a follow-up) can't ride the updateBlockAttributes + // path: their apply mutates the tree rather than a single + // block's attributes. Branch out, run the matching block- + // editor action, and short-circuit before the attribute-set + // rollback machinery below. const structuralOp = findStructuralOp( payload.operations ); if ( structuralOp ) { try { @@ -651,6 +651,25 @@ export function useSuggestionsProvider() { requestInterceptorBypass( targetClientId ); clearOverlay( targetClientId ); removeBlock( targetClientId ); + } else if ( structuralOp.type === 'block-insert-after' ) { + // The block is already in the live tree (the user + // inserted it during Suggest mode); apply just + // clears the pending-insert marker so the block + // loses its dimmed treatment. Any attribute-set + // ops in the same payload represent edits the + // user made between insertion and auto-save — + // the live block already has those values, so the + // fall-through attribute-set apply path below + // would be a near-no-op. Short-circuit here for + // clarity. + const clearAttrs = clearSuggestionMarkerAttributes( + selectBlockAttributes( targetClientId ) + ); + if ( clearAttrs ) { + requestInterceptorBypass( targetClientId ); + updateBlockAttributes( targetClientId, clearAttrs ); + } + clearOverlay( targetClientId ); } await saveEntityRecord( @@ -780,19 +799,28 @@ export function useSuggestionsProvider() { */ const rejectSuggestion = useCallback( async ( { commentId, clientId, payload } ) => { - // Clear the live block's suggestion marker for structural - // rejects. Attribute-set suggestions don't carry a marker, so - // nothing to clear. + // Reject behavior depends on the structural op type: + // - block-remove: drop the marker (block stays). + // - block-insert-after: dispatch removeBlock to undo the + // suggested insertion. The marker on the live block goes + // away with the block itself. + // - attribute-set (no structural op): no live-block change. const structuralOp = findStructuralOp( payload?.operations ); if ( structuralOp && clientId ) { - const clearAttrs = clearSuggestionMarkerAttributes( - selectBlockAttributes( clientId ) - ); - if ( clearAttrs ) { + if ( structuralOp.type === 'block-insert-after' ) { requestInterceptorBypass( clientId ); - updateBlockAttributes( clientId, clearAttrs ); + clearOverlay( clientId ); + removeBlock( clientId ); + } else { + const clearAttrs = clearSuggestionMarkerAttributes( + selectBlockAttributes( clientId ) + ); + if ( clearAttrs ) { + requestInterceptorBypass( clientId ); + updateBlockAttributes( clientId, clearAttrs ); + } + clearOverlay( clientId ); } - clearOverlay( clientId ); } try { @@ -824,6 +852,7 @@ export function useSuggestionsProvider() { createNotice, selectBlockAttributes, updateBlockAttributes, + removeBlock, requestInterceptorBypass, clearOverlay, ] diff --git a/packages/editor/src/components/suggestion-mode/store-interceptor.js b/packages/editor/src/components/suggestion-mode/store-interceptor.js index e285f598999ba6..daad4aaa7f74a5 100644 --- a/packages/editor/src/components/suggestion-mode/store-interceptor.js +++ b/packages/editor/src/components/suggestion-mode/store-interceptor.js @@ -28,8 +28,15 @@ * keyboard handlers, external integrations) that wouldn't trigger a * `useSelect`-based watcher. * - * New blocks (no snapshot entry) are tracked but not intercepted — inserting - * a block in Suggest mode is currently a real edit, not a suggestion. + * Structural changes (#77434) flow through the same subscribe loop: + * - Removed blocks → re-inserted from the previous-tick snapshot at their + * prior parent + index, then tagged `metadata.suggestion = pending-remove`. + * - New blocks → tagged `metadata.suggestion = pending-insert` (the block + * stays in the tree; the marker drives the dimmed visual treatment). + * - Move detection ships in a follow-up. + * + * In every case the live block carries the marker and a corresponding + * structural op is written to the overlay so auto-save persists it. */ /** * WordPress dependencies @@ -542,9 +549,63 @@ export default function SuggestionStoreInterceptor() { } if ( previous === undefined ) { - // New block (inserted after Suggest activated). Track it - // but don't intercept. + // New block (inserted after Suggest mode activated): + // route through the apply-and-tag flow. The block stays + // in the live tree; auto-save persists a `block-insert- + // after` op against the previous-tick tree snapshot. + // Apply later just clears the marker (the block is + // already there); Reject runs `removeBlock` to undo. + // + // Skip descendants of another new block — a Group with + // nested children fires multiple new-block entries in a + // single tick, but only the top-level Group is the + // suggested insertion. The previous-tick tree is the + // reference because the active `snapshot` map is being + // built up in parents-first iteration order, so its + // presence wouldn't distinguish "pre-existing" from + // "already-processed-new-block" parents. snapshot.set( clientId, current ); + const parentClientId = + blockEditor.getBlockRootClientId?.( clientId ) || null; + const parentExisted = + parentClientId === null || + tree.blocksByClientId.has( parentClientId ); + if ( ! parentExisted ) { + continue; + } + const block = blockEditor.getBlock?.( clientId ); + if ( ! block ) { + continue; + } + const siblingIds = + blockEditor.getBlockOrder?.( + parentClientId ?? undefined + ) ?? []; + const indexInParent = siblingIds.indexOf( clientId ); + const anchorClientId = + indexInParent > 0 + ? siblingIds[ indexInParent - 1 ] + : null; + + isReverting = true; + try { + blockEditorDispatch.updateBlockAttributes( clientId, { + metadata: withSuggestionMarker( current?.metadata, { + type: 'pending-insert', + } ), + } ); + } finally { + isReverting = false; + } + + setStructuralOpRef.current?.( clientId, block.name, { + type: 'block-insert-after', + clientId, + blockName: block.name, + anchorClientId, + parentClientId, + block, + } ); continue; } diff --git a/packages/editor/src/components/suggestion-mode/test/store-interceptor.js b/packages/editor/src/components/suggestion-mode/test/store-interceptor.js index a3da4df54d54b7..a1b924b74d4a76 100644 --- a/packages/editor/src/components/suggestion-mode/test/store-interceptor.js +++ b/packages/editor/src/components/suggestion-mode/test/store-interceptor.js @@ -454,6 +454,101 @@ describe( 'SuggestionStoreInterceptor (integration)', () => { 'pending-remove' ); } ); + + it( 'tags a newly-inserted block with metadata.suggestion = pending-insert and writes a block-insert-after op', async () => { + // Inserting a block in Suggest mode now flows through the apply- + // and-tag pipeline: the block stays at its inserted position; the + // marker drives the dimmed visual treatment; auto-save persists + // the structural op as a `block-insert-after` comment. Apply + // later just clears the marker, Reject runs `removeBlock` to + // undo. + const { registry, clientId, getOverlay } = setup(); + + const inserted = createBlock( TEST_BLOCK_NAME, { content: 'New' } ); + await act( async () => { + registry + .dispatch( blockEditorStore ) + .insertBlock( inserted, 1, undefined, false ); + } ); + await flushSubscribers(); + + const liveBlocks = registry.select( blockEditorStore ).getBlocks(); + expect( liveBlocks ).toHaveLength( 2 ); + const newBlock = liveBlocks[ 1 ]; + expect( newBlock.clientId ).toBe( inserted.clientId ); + expect( newBlock.attributes?.metadata?.suggestion ).toEqual( { + type: 'pending-insert', + } ); + + expect( + getOverlay().entries[ inserted.clientId ]?.structuralOp + ).toMatchObject( { + type: 'block-insert-after', + clientId: inserted.clientId, + blockName: TEST_BLOCK_NAME, + anchorClientId: clientId, + parentClientId: null, + } ); + } ); + + it( 'records a null anchor when the inserted block lands at index 0 (no previous sibling)', async () => { + const { registry, getOverlay } = setup(); + + const inserted = createBlock( TEST_BLOCK_NAME, { content: 'First' } ); + await act( async () => { + registry + .dispatch( blockEditorStore ) + .insertBlock( inserted, 0, undefined, false ); + } ); + await flushSubscribers(); + + const liveBlocks = registry.select( blockEditorStore ).getBlocks(); + expect( liveBlocks[ 0 ].clientId ).toBe( inserted.clientId ); + expect( liveBlocks[ 0 ].attributes?.metadata?.suggestion?.type ).toBe( + 'pending-insert' + ); + expect( + getOverlay().entries[ inserted.clientId ]?.structuralOp + ).toMatchObject( { + type: 'block-insert-after', + anchorClientId: null, + parentClientId: null, + } ); + } ); + + it( 'tags only the top-level new block when an inserted subtree contains nested children', async () => { + // A Group block with a child paragraph dispatches both as new in + // the same tick. The interceptor must tag only the top-level + // Group as pending-insert; the child rides along inside the + // captured snapshot. + const { registry } = setup(); + + const child = createBlock( TEST_BLOCK_NAME, { content: 'Child' } ); + const parent = createBlock( TEST_BLOCK_NAME, { content: 'Parent' }, [ + child, + ] ); + await act( async () => { + registry + .dispatch( blockEditorStore ) + .insertBlock( parent, 1, undefined, false ); + } ); + await flushSubscribers(); + + const insertedRoot = registry + .select( blockEditorStore ) + .getBlock( parent.clientId ); + expect( insertedRoot?.attributes?.metadata?.suggestion?.type ).toBe( + 'pending-insert' + ); + const insertedChild = registry + .select( blockEditorStore ) + .getBlock( child.clientId ); + // Child must NOT be tagged — its parent's marker covers the whole + // subtree. + expect( + insertedChild?.attributes?.metadata?.suggestion + ).toBeUndefined(); + } ); } ); describe( 'isAcceptedSuggestionChange', () => { From 556596d40afb68a19c815d61d44a31b62263d6c8 Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Tue, 5 May 2026 13:18:09 -0700 Subject: [PATCH 2/5] Suggestions: Re-seed snapshot for re-inserted removed blocks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A `removeBlock` dispatch in Suggest mode runs through the apply-and-tag flow: re-insert the subtree at its previous position, tag the top with `metadata.suggestion = pending-remove`, and write a `block-remove` op into the overlay. The post-handler used to call `snapshot.delete` on the removed clientIds — fine in isolation, but once the new-block branch (this PR) starts tagging unfamiliar blocks as `pending-insert`, any follow-up subscribe fire (a YJS sync echo, an unrelated dispatch, or a follow-up tick drained by React's batching) finds the re-inserted block missing from the snapshot, routes it through that branch, and overwrites both the marker and the persisted structural op with the insert pair. Re-seed `snapshot` from the live attributes instead. The pending- remove marker is in `metadata.suggestion`, which is already a member of `SYSTEM_METADATA_KEYS`, so the re-seeded baseline keeps the marker invisible to subsequent diffs without leaking it into the user-pending overlay. Refs #77434. --- .../suggestion-mode/store-interceptor.js | 24 +++++++-- .../suggestion-mode/test/store-interceptor.js | 50 +++++++++++++++++++ 2 files changed, 70 insertions(+), 4 deletions(-) diff --git a/packages/editor/src/components/suggestion-mode/store-interceptor.js b/packages/editor/src/components/suggestion-mode/store-interceptor.js index daad4aaa7f74a5..19e5cba5485b7d 100644 --- a/packages/editor/src/components/suggestion-mode/store-interceptor.js +++ b/packages/editor/src/components/suggestion-mode/store-interceptor.js @@ -763,11 +763,27 @@ export default function SuggestionStoreInterceptor() { } ); } - // Drop snapshot entries for any remaining (descendant) - // removed blocks — they came back as part of their parent's - // re-inserted subtree, but we still want a fresh re-seed. + // Re-seed the snapshot for every block that came back via + // the re-insert (top-level tops AND their descendants). The + // live attributes for tops include the pending-remove + // marker; `metadata.suggestion`'s membership in + // SYSTEM_METADATA_KEYS keeps the marker invisible to the + // next diff. Using `snapshot.delete` here would leave the + // re-inserted blocks looking new, so the next subscribe + // fire — triggered by a sync echo, an unrelated dispatch, + // or React batching draining a follow-up tick — would route + // them through the new-block branch and overwrite both the + // pending-remove marker and the persisted `block-remove` + // structural op with the `pending-insert` / + // `block-insert-after` pair. for ( const clientId of removedIds ) { - snapshot.delete( clientId ); + const liveAttrs = + blockEditor.getBlockAttributes?.( clientId ); + if ( liveAttrs ) { + snapshot.set( clientId, liveAttrs ); + } else { + snapshot.delete( clientId ); + } } } diff --git a/packages/editor/src/components/suggestion-mode/test/store-interceptor.js b/packages/editor/src/components/suggestion-mode/test/store-interceptor.js index a1b924b74d4a76..c0277b5e87a503 100644 --- a/packages/editor/src/components/suggestion-mode/test/store-interceptor.js +++ b/packages/editor/src/components/suggestion-mode/test/store-interceptor.js @@ -427,6 +427,56 @@ describe( 'SuggestionStoreInterceptor (integration)', () => { ); } ); + it( 'keeps the pending-remove marker after a follow-up dispatch fires the subscribe loop again', async () => { + // Regression: a follow-up store update (a sync echo, an unrelated + // dispatch, or React batching draining a second tick) used to find + // the re-inserted block missing from the snapshot and route it + // through the new-block branch — overwriting the pending-remove + // marker with pending-insert and the structural op from + // `block-remove` to `block-insert-after`. + const a = createBlock( TEST_BLOCK_NAME, { content: 'A' } ); + const b = createBlock( TEST_BLOCK_NAME, { content: 'B' } ); + const c = createBlock( TEST_BLOCK_NAME, { content: 'C' } ); + const { registry, getOverlay } = setup( { + initialBlocks: [ a, b, c ], + } ); + + await act( async () => { + registry.dispatch( blockEditorStore ).removeBlock( b.clientId ); + } ); + await flushSubscribers(); + + // Sanity: the first fire tagged the re-inserted block as + // pending-remove and recorded the matching structural op. + expect( + registry.select( blockEditorStore ).getBlockAttributes( b.clientId ) + ?.metadata?.suggestion?.type + ).toBe( 'pending-remove' ); + expect( getOverlay().entries[ b.clientId ]?.structuralOp?.type ).toBe( + 'block-remove' + ); + + // Trigger a second subscribe fire by dispatching an unrelated + // store update. This is the same code path a YJS sync echo or a + // follow-up editor action exercises in the wild. + await act( async () => { + registry + .dispatch( blockEditorStore ) + .updateBlockAttributes( a.clientId, { content: 'A!' } ); + } ); + await flushSubscribers(); + + // The re-inserted block must still carry the pending-remove + // marker and its structural op must still describe a removal. + expect( + registry.select( blockEditorStore ).getBlockAttributes( b.clientId ) + ?.metadata?.suggestion?.type + ).toBe( 'pending-remove' ); + expect( getOverlay().entries[ b.clientId ]?.structuralOp?.type ).toBe( + 'block-remove' + ); + } ); + it( 're-inserts only the top-level removed block when a parent and its child are removed together', async () => { // `removeBlock` on a parent removes the whole subtree atomically. // We must re-insert just the parent — its child rides along. From 8ccd073fd409aab529b0866c85c1ee35f541f058 Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Tue, 5 May 2026 13:18:26 -0700 Subject: [PATCH 3/5] Suggestions: Apply attribute-set ops when accepting a block-insert-after MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The block-insert-after apply branch only cleared the pending-insert marker on the live block, on the assumption that the live block already carried the suggested values. That holds on the suggester's own client — but the suggester's interceptor diverts every post- insertion edit into the overlay and reverts the live attributes, so collaborators (and the suggester after a reload) see the live block in its captured-at-insertion shape: typically an empty paragraph. After Apply, the marker disappears but the block stays empty, which reads as "the inserted block didn't appear." Fold the payload's attribute-set ops into the apply payload along with the marker clear: `applyOperations` produces the merged attributes, `clearSuggestionMarkerAttributes` strips the marker from the result, and the two are spread together for a single `updateBlockAttributes` dispatch. The marker-clear path in `adoptSystemMetadata` keeps the dispatch from looping back through the interceptor. Refs #77434. --- .../components/suggestion-mode/provider.js | 45 ++++++++++++------ .../suggestion-mode/test/provider.js | 47 +++++++++++++++++++ 2 files changed, 77 insertions(+), 15 deletions(-) diff --git a/packages/editor/src/components/suggestion-mode/provider.js b/packages/editor/src/components/suggestion-mode/provider.js index a6ac34997b7cd2..bc2e81fc65aa5f 100644 --- a/packages/editor/src/components/suggestion-mode/provider.js +++ b/packages/editor/src/components/suggestion-mode/provider.js @@ -653,22 +653,37 @@ export function useSuggestionsProvider() { removeBlock( targetClientId ); } else if ( structuralOp.type === 'block-insert-after' ) { // The block is already in the live tree (the user - // inserted it during Suggest mode); apply just - // clears the pending-insert marker so the block - // loses its dimmed treatment. Any attribute-set - // ops in the same payload represent edits the - // user made between insertion and auto-save — - // the live block already has those values, so the - // fall-through attribute-set apply path below - // would be a near-no-op. Short-circuit here for - // clarity. - const clearAttrs = clearSuggestionMarkerAttributes( - selectBlockAttributes( targetClientId ) + // inserted it during Suggest mode); apply commits + // the captured edits onto the live block AND clears + // the pending-insert marker so the block loses its + // dimmed treatment. + // + // Attribute-set ops in the same payload represent + // edits the user made between insertion and auto- + // save. They never reach the live block on the + // suggester's side — the interceptor reverts them + // into the overlay — so collaborators (and the + // suggester after a reload) see the live block in + // the captured shape (typically empty content for + // a fresh paragraph). Apply must materialize those + // edits on the live block, otherwise the inserted + // block ends up empty after acceptance. + const currentAttributes = + selectBlockAttributes( targetClientId ); + const withOpsApplied = applyOperations( + currentAttributes, + payload.operations + ); + const markerCleared = + clearSuggestionMarkerAttributes( withOpsApplied ); + const finalAttributes = markerCleared + ? { ...withOpsApplied, ...markerCleared } + : withOpsApplied; + requestInterceptorBypass( targetClientId ); + updateBlockAttributes( + targetClientId, + finalAttributes ); - if ( clearAttrs ) { - requestInterceptorBypass( targetClientId ); - updateBlockAttributes( targetClientId, clearAttrs ); - } clearOverlay( targetClientId ); } diff --git a/packages/editor/src/components/suggestion-mode/test/provider.js b/packages/editor/src/components/suggestion-mode/test/provider.js index b79d59600553a2..3e7e14bd6be1c5 100644 --- a/packages/editor/src/components/suggestion-mode/test/provider.js +++ b/packages/editor/src/components/suggestion-mode/test/provider.js @@ -132,6 +132,53 @@ describe( 'applyOperations', () => { expect( result ).toEqual( attrs ); expect( result ).not.toBe( attrs ); } ); + + it( 'composes with clearSuggestionMarkerAttributes to produce the inserted-block apply payload', () => { + // When applying a block-insert-after suggestion the live block on + // the accepter's side is in its captured-at-insertion shape — the + // suggester's interceptor diverted any subsequent typing into the + // overlay rather than committing it. Apply must (1) materialize + // those overlay edits as attribute-set ops, AND (2) clear the + // pending-insert marker. Combining `applyOperations` with + // `clearSuggestionMarkerAttributes` yields the merged payload the + // provider hands to `updateBlockAttributes`. + const liveAttributes = { + content: '', + metadata: { + noteId: [ 42 ], + suggestion: { type: 'pending-insert' }, + }, + }; + const operations = [ + { + type: 'block-insert-after', + clientId: 'abc', + blockName: 'core/paragraph', + anchorClientId: null, + parentClientId: null, + block: { name: 'core/paragraph', attributes: { content: '' } }, + }, + { + type: 'attribute-set', + attribute: 'content', + before: '', + after: 'Hello world', + }, + ]; + + const withOpsApplied = applyOperations( liveAttributes, operations ); + const markerCleared = clearSuggestionMarkerAttributes( withOpsApplied ); + const finalAttributes = markerCleared + ? { ...withOpsApplied, ...markerCleared } + : withOpsApplied; + + // The typed content is committed to the live block... + expect( finalAttributes.content ).toBe( 'Hello world' ); + // ...and the pending-insert marker is gone, while noteId + // (system metadata) is preserved. + expect( finalAttributes.metadata ).toEqual( { noteId: [ 42 ] } ); + expect( finalAttributes.metadata.suggestion ).toBeUndefined(); + } ); } ); describe( 'payloadByteLength', () => { From 203de589b5630dfccdcd8f68f9425561ea3b13bc Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Tue, 5 May 2026 15:37:13 -0700 Subject: [PATCH 4/5] Suggestions: Adopt synced reject of a pending-insert instead of re-tagging MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rejecting a pending-insert suggestion dispatches a `removeBlock` to undo the insert. When that lands on a peer client through sync, batched with the marker-clearing `updateBlockAttributes`, the removal-detection branch was treating the disappearance as a fresh user delete — re-inserting the block and tagging it pending-remove. That re-insert bounced back through sync and undid the reject on the rejecting client a moment after they clicked. Extend the apply-landing check to also recognize `pending-insert` in the previous-tick tree snapshot. The marker presence means the removal is the reject landing; adopt it. --- .../suggestion-mode/store-interceptor.js | 37 ++++++++----- .../suggestion-mode/test/store-interceptor.js | 55 +++++++++++++++++++ 2 files changed, 77 insertions(+), 15 deletions(-) diff --git a/packages/editor/src/components/suggestion-mode/store-interceptor.js b/packages/editor/src/components/suggestion-mode/store-interceptor.js index 427cb4308bcfe0..81c52e3d078314 100644 --- a/packages/editor/src/components/suggestion-mode/store-interceptor.js +++ b/packages/editor/src/components/suggestion-mode/store-interceptor.js @@ -695,24 +695,31 @@ export default function SuggestionStoreInterceptor() { const removedIds = []; for ( const clientId of tree.blocksByClientId.keys() ) { if ( ! live.has( clientId ) ) { - // "Apply landing": when another client accepts a - // `pending-remove` suggestion, the resulting - // `removeBlock` arrives here through sync — - // typically batched with the marker-clearing - // `updateBlockAttributes` into a single block- - // editor update, which fires this subscriber - // once. The previous-tick tree snapshot still - // carries the pending marker, so we can recognize - // the disappearance as the apply landing rather - // than a fresh user delete. Adopt the removal: - // drop the snapshot entry and skip the re-insert - // + re-tag path. Without this, the apply bounces - // back through sync and undoes the removal on the - // accepting client a moment after they clicked. + // "Apply / reject landing": when another client + // accepts a `pending-remove` — or rejects a + // `pending-insert`, which also dispatches + // `removeBlock` to undo the insert — the + // resulting `removeBlock` arrives here through + // sync, typically batched with the marker- + // clearing `updateBlockAttributes` into a + // single block-editor update, which fires + // this subscriber once. The previous-tick + // tree snapshot still carries the pending + // marker, so we can recognize the + // disappearance as the suggestion landing + // rather than a fresh user delete. Adopt the + // removal: drop the snapshot entry and skip + // the re-insert + re-tag path. Without this, + // the apply / reject bounces back through + // sync and undoes the change on the accepting + // client a moment after they clicked. const trackedMarker = tree.blocksByClientId.get( clientId )?.attributes ?.metadata?.suggestion?.type; - if ( trackedMarker === 'pending-remove' ) { + if ( + trackedMarker === 'pending-remove' || + trackedMarker === 'pending-insert' + ) { snapshot.delete( clientId ); continue; } diff --git a/packages/editor/src/components/suggestion-mode/test/store-interceptor.js b/packages/editor/src/components/suggestion-mode/test/store-interceptor.js index af57d19980d687..4532a427abdd59 100644 --- a/packages/editor/src/components/suggestion-mode/test/store-interceptor.js +++ b/packages/editor/src/components/suggestion-mode/test/store-interceptor.js @@ -592,6 +592,61 @@ describe( 'SuggestionStoreInterceptor (integration)', () => { } ); } ); + it( 'adopts a removal that arrives bundled with its marker-clear (reject of pending-insert via sync)', async () => { + // Regression: rejecting a pending-insert suggestion on + // another client dispatches `removeBlock` (the rejection + // undoes the insert). When that arrives here via sync, + // batched with the corresponding marker-clear, the + // disappearing block carries `metadata.suggestion.type === + // 'pending-insert'` in the previous-tick tree. The + // interceptor must adopt that as the reject landing instead + // of re-inserting the block and re-tagging it as a fresh + // pending-remove — otherwise the rejected paragraph reappears + // on the suggester's screen a moment later via the bounced + // re-insert. + const a = createBlock( TEST_BLOCK_NAME, { content: 'A' } ); + const inserted = createBlock( TEST_BLOCK_NAME, { content: 'New' } ); + const { registry, getOverlay } = setup( { initialBlocks: [ a ] } ); + + // First, this client creates the pending-insert suggestion. + await act( async () => { + registry + .dispatch( blockEditorStore ) + .insertBlock( inserted, 1, undefined, false ); + } ); + await flushSubscribers(); + + expect( + registry + .select( blockEditorStore ) + .getBlockAttributes( inserted.clientId )?.metadata?.suggestion + ?.type + ).toBe( 'pending-insert' ); + + // Simulate the reject landing: marker-clear + removeBlock + // batched into a single store update. + await act( async () => { + registry.batch( () => { + registry + .dispatch( blockEditorStore ) + .updateBlockAttributes( inserted.clientId, { + metadata: {}, + } ); + registry + .dispatch( blockEditorStore ) + .removeBlock( inserted.clientId ); + } ); + } ); + await flushSubscribers(); + + // The block must stay removed — the interceptor adopts the + // reject landing rather than re-inserting and re-tagging. + const liveBlocks = registry.select( blockEditorStore ).getBlocks(); + expect( liveBlocks ).toHaveLength( 1 ); + expect( liveBlocks[ 0 ].clientId ).toBe( a.clientId ); + expect( getOverlay().entries[ inserted.clientId ] ).toBeUndefined(); + } ); + it( 'records a null anchor when the inserted block lands at index 0 (no previous sibling)', async () => { const { registry, getOverlay } = setup(); From 5d8bf4c9b168dfd178192df752c0c88cc69c1876 Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Tue, 5 May 2026 16:02:00 -0700 Subject: [PATCH 5/5] Suggestions: Skip staleness prompt when applying a block-insert-after MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The conflict check compares each attribute-set op's `before` against the live block's current attribute. For an inserted block the overlay baseline is `{}`, so every attribute-set op rides on `before: null`. On the accepting client the inserted block already carries the typed content, so the comparison reads as a divergence and the apply flow opens a 'This block has changed' dialog before every Insert apply. Inserted blocks have no pre-existing attributes to overwrite — the attribute-set ops describe the new block's content, not concurrent edits. Short-circuit `hasAttributeConflict` to false when the payload carries a `block-insert-after` structural op, and add a regression test. --- .../components/suggestion-mode/provider.js | 10 ++++++ .../suggestion-mode/test/provider.js | 31 +++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/packages/editor/src/components/suggestion-mode/provider.js b/packages/editor/src/components/suggestion-mode/provider.js index bc2e81fc65aa5f..d69f52916fbdb4 100644 --- a/packages/editor/src/components/suggestion-mode/provider.js +++ b/packages/editor/src/components/suggestion-mode/provider.js @@ -265,6 +265,16 @@ export function hasAttributeConflict( currentAttributes, operations ) { if ( ! Array.isArray( operations ) ) { return false; } + // Inserted blocks have no pre-existing attributes — the overlay's + // baseline for a `block-insert-after` entry is `{}`, so every + // attribute-set op rides on `before: null`. Comparing that against the + // live (already-typed-into) block's attributes always reads as + // divergence, which falsely fires the staleness prompt on apply. The + // attribute-set ops describe the inserted block's content, not an + // overwrite of pre-existing data, so there is nothing to conflict with. + if ( findStructuralOp( operations )?.type === 'block-insert-after' ) { + return false; + } for ( const op of operations ) { if ( op.type !== 'attribute-set' ) { continue; diff --git a/packages/editor/src/components/suggestion-mode/test/provider.js b/packages/editor/src/components/suggestion-mode/test/provider.js index 3e7e14bd6be1c5..630ef42ef4b2f2 100644 --- a/packages/editor/src/components/suggestion-mode/test/provider.js +++ b/packages/editor/src/components/suggestion-mode/test/provider.js @@ -295,6 +295,37 @@ describe( 'hasAttributeConflict', () => { hasAttributeConflict( { content: wrapperOther }, [ CONTENT_OP ] ) ).toBe( true ); } ); + + it( 'returns false for a block-insert-after payload even when attribute-set ops appear divergent', () => { + // The overlay baseline for an inserted block is `{}` — every + // attribute-set op the auto-save loop persists carries + // `before: null` regardless of what the user typed. Comparing + // that against the live (already-typed-into) block on the + // accepting client must not fire the staleness prompt. + const operations = [ + { + type: 'block-insert-after', + clientId: 'inserted', + blockName: 'core/paragraph', + anchorClientId: 'anchor', + parentClientId: null, + block: { + name: 'core/paragraph', + attributes: { content: 'Hi' }, + innerBlocks: [], + }, + }, + { + type: 'attribute-set', + attribute: 'content', + before: null, + after: 'Hi', + }, + ]; + expect( hasAttributeConflict( { content: 'Hi' }, operations ) ).toBe( + false + ); + } ); } ); describe( 'parseSuggestionPayload', () => {