fix: yjs sync updates being blocked by locked sdt and charts (#2506)

* fix: allow Yjs sync through locked content while preserving chart immutability

* fix: skip permission-tag repair for Yjs replacements and preserve chart fast path

* fix: treat y-prosemirror snapshot exits as authoritative updates

Author: harbournick

packages/super-editor/src/extensions/chart/chart-immutability-plugin.js

@@ -1,4 +1,5 @@
 import { Plugin, PluginKey } from 'prosemirror-state';
+import { ySyncPluginKey } from 'y-prosemirror';
 
 export const CHART_IMMUTABILITY_KEY = new PluginKey('chartImmutability');
 
@@ -122,15 +123,26 @@ export function createChartImmutabilityPlugin() {
       init(_, state) {
         return countChartNodes(state.doc);
       },
-      apply(_tr, oldCount) {
-        // filterTransaction guarantees no chart mutations passed through,
-        // so the chart count is always unchanged after init.
+      apply(tr, oldCount, _oldState, newState) {
+        // Yjs-origin transactions bypass filterTransaction, so the chart
+        // count may have changed. Recount to keep the fast-path guard
+        // (oldCount === 0) accurate after collaborative syncs.
+        if (tr.docChanged && tr.getMeta?.(ySyncPluginKey)) {
+          // When the document had no charts, only do a full recount if the
+          // incoming steps actually contain a chart node. This preserves
+          // O(step slices) cost for text-only remote edits on chart-free docs.
+          if (oldCount === 0 && !transactionInsertsChart(tr)) {
+            return 0;
+          }
+          return countChartNodes(newState.doc);
+        }
         return oldCount;
       },
     },
 
     filterTransaction(tr, state) {
       if (!tr.docChanged) return true;
+      if (tr.getMeta?.(ySyncPluginKey)) return true;
 
       const oldCount = CHART_IMMUTABILITY_KEY.getState(state) ?? 0;
       if (oldCount === 0) {

packages/super-editor/src/extensions/chart/chart-immutability-plugin.test.js

@@ -1,7 +1,8 @@
 import { describe, it, expect } from 'vitest';
-import { Schema } from 'prosemirror-model';
+import { Schema, Slice } from 'prosemirror-model';
 import { EditorState } from 'prosemirror-state';
-import { createChartImmutabilityPlugin } from './chart-immutability-plugin.js';
+import { ySyncPluginKey } from 'y-prosemirror';
+import { createChartImmutabilityPlugin, CHART_IMMUTABILITY_KEY } from './chart-immutability-plugin.js';
 
 const schema = new Schema({
   nodes: {
@@ -107,4 +108,123 @@ describe('chart immutability plugin', () => {
     const result = state.applyTransaction(tr);
     expect(result.state.doc.textContent).toContain('typing');
   });
+
+  it('allows remote collaboration replacements that span chart nodes', () => {
+    const state = createStateWithChart();
+    const replacementDoc = schema.nodes.doc.create(null, [schema.nodes.paragraph.create(null, schema.text('remote'))]);
+    const tr = state.tr
+      .replace(0, state.doc.content.size, new Slice(replacementDoc.content, 0, 0))
+      .setMeta(ySyncPluginKey, { isChangeOrigin: true });
+
+    const result = state.applyTransaction(tr);
+
+    expect(result.state.doc.textContent).toContain('remote');
+  });
+
+  it('allows snapshot-exit replacements that span chart nodes (no isChangeOrigin)', () => {
+    const state = createStateWithChart();
+    const replacementDoc = schema.nodes.doc.create(null, [
+      schema.nodes.paragraph.create(null, schema.text('snapshot exit')),
+    ]);
+    // y-prosemirror's unrenderSnapshot() sets { snapshot: null, prevSnapshot: null }
+    // with no isChangeOrigin flag.
+    const tr = state.tr
+      .replace(0, state.doc.content.size, new Slice(replacementDoc.content, 0, 0))
+      .setMeta(ySyncPluginKey, { snapshot: null, prevSnapshot: null });
+
+    const result = state.applyTransaction(tr);
+
+    expect(result.state.doc.textContent).toContain('snapshot exit');
+  });
+
+  it('rejects local chart deletion after Yjs sync inserts a chart', () => {
+    // Start with no charts
+    const state = createStateWithoutChart();
+
+    // Simulate Yjs sync that introduces a chart
+    const chart = schema.nodes.chart.create({ chartData: { chartType: 'barChart', series: [] } });
+    const paraWithChart = schema.nodes.paragraph.create(null, [schema.text('synced '), chart]);
+    const syncDoc = schema.nodes.doc.create(null, [paraWithChart]);
+    const syncTr = state.tr
+      .replace(0, state.doc.content.size, new Slice(syncDoc.content, 0, 0))
+      .setMeta(ySyncPluginKey, { isChangeOrigin: true });
+    const synced = state.applyTransaction(syncTr).state;
+
+    // Local attempt to delete the chart must be rejected
+    const chartPos = findChartPos(synced);
+    expect(chartPos).toBeGreaterThan(-1);
+    const deleteTr = synced.tr.delete(chartPos, chartPos + 1);
+    const result = synced.applyTransaction(deleteTr);
+    expect(result.state.doc.toString()).toBe(synced.doc.toString());
+  });
+
+  it('rejects local attr change on chart after Yjs sync inserts it', () => {
+    const state = createStateWithoutChart();
+
+    // Yjs sync introduces a chart
+    const chart = schema.nodes.chart.create({ chartData: { chartType: 'barChart', series: [] } });
+    const paraWithChart = schema.nodes.paragraph.create(null, [chart]);
+    const syncDoc = schema.nodes.doc.create(null, [paraWithChart]);
+    const syncTr = state.tr
+      .replace(0, state.doc.content.size, new Slice(syncDoc.content, 0, 0))
+      .setMeta(ySyncPluginKey, { isChangeOrigin: true });
+    const synced = state.applyTransaction(syncTr).state;
+
+    // Local setNodeMarkup must be rejected
+    const chartPos = findChartPos(synced);
+    const attrTr = synced.tr.setNodeMarkup(chartPos, undefined, {
+      chartData: { chartType: 'lineChart', series: [] },
+      width: 999,
+      height: 999,
+    });
+    const result = synced.applyTransaction(attrTr);
+    expect(result.state.doc.toString()).toBe(synced.doc.toString());
+  });
+
+  it('uses fast path after Yjs sync removes all charts', () => {
+    const state = createStateWithChart();
+
+    // Yjs sync replaces doc with chart-free content
+    const plainDoc = schema.nodes.doc.create(null, [schema.nodes.paragraph.create(null, schema.text('no charts'))]);
+    const syncTr = state.tr
+      .replace(0, state.doc.content.size, new Slice(plainDoc.content, 0, 0))
+      .setMeta(ySyncPluginKey, { isChangeOrigin: true });
+    const synced = state.applyTransaction(syncTr).state;
+
+    // Local text edits should still work
+    const textTr = synced.tr.insertText('hello', 1);
+    const result = synced.applyTransaction(textTr);
+    expect(result.state.doc.textContent).toContain('hello');
+  });
+
+  it('keeps chart count at 0 for Yjs text-only edits on chart-free docs (fast path)', () => {
+    const state = createStateWithoutChart();
+    expect(CHART_IMMUTABILITY_KEY.getState(state)).toBe(0);
+
+    // Simulate multiple Yjs text-only remote edits — chart count must stay 0
+    // without walking the full document each time.
+    let current = state;
+    for (let i = 0; i < 5; i++) {
+      const syncTr = current.tr.insertText(`k${i}`, 1).setMeta(ySyncPluginKey, { isChangeOrigin: true });
+      current = current.applyTransaction(syncTr).state;
+      expect(CHART_IMMUTABILITY_KEY.getState(current)).toBe(0);
+    }
+  });
+
+  it('recounts charts when Yjs sync introduces a chart into a chart-free doc', () => {
+    const state = createStateWithoutChart();
+    expect(CHART_IMMUTABILITY_KEY.getState(state)).toBe(0);
+
+    // Yjs sync introduces a chart
+    const chart = schema.nodes.chart.create({ chartData: { chartType: 'barChart', series: [] } });
+    const paraWithChart = schema.nodes.paragraph.create(null, [schema.text('text '), chart]);
+    const syncDoc = schema.nodes.doc.create(null, [paraWithChart]);
+    const syncTr = state.tr
+      .replace(0, state.doc.content.size, new Slice(syncDoc.content, 0, 0))
+      .setMeta(ySyncPluginKey, { isChangeOrigin: true });
+    const synced = state.applyTransaction(syncTr).state;
+
+    // Chart count must update to 1
+    expect(CHART_IMMUTABILITY_KEY.getState(synced)).toBe(1);
+  });
 });

packages/super-editor/src/extensions/permission-ranges/permission-ranges.js

@@ -1,5 +1,6 @@
 import { Plugin, PluginKey } from 'prosemirror-state';
 import { Mapping } from 'prosemirror-transform';
+import { ySyncPluginKey } from 'y-prosemirror';
 import { Extension } from '@core/Extension.js';
 
 const PERMISSION_PLUGIN_KEY = new PluginKey('permissionRanges');
@@ -304,6 +305,11 @@ export const PermissionRanges = Extension.create({
         // Appends transactions to the document to ensure permission ranges are updated.
         appendTransaction(transactions, oldState, newState) {
           if (!transactions.some((tr) => tr.docChanged)) return null;
+          // Any y-prosemirror transaction (remote sync, snapshot enter/exit)
+          // carries authoritative state — do not attempt to repair permission
+          // tags against oldState or we will reinsert stale markers and
+          // corrupt the shared document.
+          if (transactions.some((tr) => tr.getMeta?.(ySyncPluginKey))) return null;
 
           const permTypes = getPermissionTypeInfo(newState.schema);
           if (!permTypes.startTypes.length || !permTypes.endTypes.length) return null;
@@ -370,6 +376,7 @@ export const PermissionRanges = Extension.create({
         // Filters transactions to ensure only allowed edits are applied.
         filterTransaction(tr, state) {
           if (!tr.docChanged) return true;
+          if (tr.getMeta?.(ySyncPluginKey)) return true;
           if (!editor || editor.options.documentMode !== 'viewing') return true;
           const pluginState = PERMISSION_PLUGIN_KEY.getState(state);
           if (!pluginState?.hasAllowedRanges) {

packages/super-editor/src/extensions/permission-ranges/permission-ranges.test.js

@@ -1,5 +1,7 @@
 import { describe, it, expect, vi, afterEach, beforeEach } from 'vitest';
 import { TextSelection } from 'prosemirror-state';
+import { Slice } from 'prosemirror-model';
+import { ySyncPluginKey } from 'y-prosemirror';
 
 import { Editor } from '@core/index.js';
 import { SuperConverter } from '@core/super-converter/SuperConverter.js';
@@ -166,6 +168,91 @@ describe('PermissionRanges extension', () => {
     expect(instance.state.doc.textBetween(editablePos, editablePos + 2)).toContain('Y');
   });
 
+  it('allows remote collaboration replacements that span permission ranges', () => {
+    const instance = createEditor(docWithPermissionRange);
+    const replacementDoc = instance.schema.nodes.doc.create(null, [
+      instance.schema.nodes.paragraph.create(null, [instance.schema.text('Remote replacement')]),
+    ]);
+    const tr = instance.state.tr
+      .replace(0, instance.state.doc.content.size, new Slice(replacementDoc.content, 0, 0))
+      .setMeta(ySyncPluginKey, { isChangeOrigin: true });
+
+    instance.view.dispatch(tr);
+
+    expect(instance.state.doc.textContent).toContain('Remote replacement');
+  });
+
+  it('does not reinsert stale permission tags on snapshot-exit replace (no isChangeOrigin)', () => {
+    const instance = createEditor(docWithPermissionRange);
+
+    // Verify the original doc has permStart and permEnd
+    let hasPermStart = false;
+    let hasPermEnd = false;
+    instance.state.doc.descendants((node) => {
+      if (node.type?.name === 'permStart') hasPermStart = true;
+      if (node.type?.name === 'permEnd') hasPermEnd = true;
+    });
+    expect(hasPermStart).toBe(true);
+    expect(hasPermEnd).toBe(true);
+
+    // Simulate y-prosemirror's unrenderSnapshot() — full-doc replace with
+    // { snapshot: null, prevSnapshot: null } and NO isChangeOrigin flag.
+    const replacementDoc = instance.schema.nodes.doc.create(null, [
+      instance.schema.nodes.paragraph.create(null, [instance.schema.text('Snapshot exit content')]),
+    ]);
+    const tr = instance.state.tr
+      .replace(0, instance.state.doc.content.size, new Slice(replacementDoc.content, 0, 0))
+      .setMeta(ySyncPluginKey, { snapshot: null, prevSnapshot: null });
+
+    instance.view.dispatch(tr);
+
+    // The resulting doc must NOT contain stale permission markers
+    let permStartCount = 0;
+    let permEndCount = 0;
+    instance.state.doc.descendants((node) => {
+      if (node.type?.name === 'permStart') permStartCount++;
+      if (node.type?.name === 'permEnd') permEndCount++;
+    });
+    expect(permStartCount).toBe(0);
+    expect(permEndCount).toBe(0);
+    expect(instance.state.doc.textContent).toBe('Snapshot exit content');
+  });
+
+  it('does not reinsert stale permission tags when Yjs replaces content that had them', () => {
+    const instance = createEditor(docWithPermissionRange);
+
+    // Verify the original doc has permStart and permEnd
+    let hasPermStart = false;
+    let hasPermEnd = false;
+    instance.state.doc.descendants((node) => {
+      if (node.type?.name === 'permStart') hasPermStart = true;
+      if (node.type?.name === 'permEnd') hasPermEnd = true;
+    });
+    expect(hasPermStart).toBe(true);
+    expect(hasPermEnd).toBe(true);
+
+    // Simulate a Yjs sync that replaces the whole doc with content lacking perm tags
+    const replacementDoc = instance.schema.nodes.doc.create(null, [
+      instance.schema.nodes.paragraph.create(null, [instance.schema.text('Clean remote content')]),
+    ]);
+    const tr = instance.state.tr
+      .replace(0, instance.state.doc.content.size, new Slice(replacementDoc.content, 0, 0))
+      .setMeta(ySyncPluginKey, { isChangeOrigin: true });
+
+    instance.view.dispatch(tr);
+
+    // The resulting doc must NOT contain stale permission markers
+    let permStartCount = 0;
+    let permEndCount = 0;
+    instance.state.doc.descendants((node) => {
+      if (node.type?.name === 'permStart') permStartCount++;
+      if (node.type?.name === 'permEnd') permEndCount++;
+    });
+    expect(permStartCount).toBe(0);
+    expect(permEndCount).toBe(0);
+    expect(instance.state.doc.textContent).toBe('Clean remote content');
+  });
+
   it('blocks edits outside the block permission range but allows edits inside it', () => {
     const instance = createEditor(docWithBlockPermissionRange);
     const initialJson = instance.state.doc.toJSON();

packages/super-editor/src/extensions/structured-content/structured-content-lock-plugin.js

@@ -1,4 +1,5 @@
 import { Plugin, PluginKey } from 'prosemirror-state';
+import { ySyncPluginKey } from 'y-prosemirror';
 
 export const STRUCTURED_CONTENT_LOCK_KEY = new PluginKey('structuredContentLock');
 
@@ -159,6 +160,13 @@ export function createStructuredContentLockPlugin() {
         return true;
       }
 
+      // Any y-prosemirror transaction (remote sync, snapshot enter/exit) must
+      // always be applied locally to keep every client converged, even if the
+      // incoming step spans locked SDTs.
+      if (tr.getMeta?.(ySyncPluginKey)) {
+        return true;
+      }
+
       const sdtNodes = STRUCTURED_CONTENT_LOCK_KEY.getState(state);
       if (sdtNodes.length === 0) {
         return true;

packages/super-editor/src/extensions/structured-content/structured-content-lock-plugin.test.js

@@ -1,5 +1,7 @@
 import { describe, it, expect, beforeEach, afterEach } from 'vitest';
 import { EditorState, TextSelection } from 'prosemirror-state';
+import { Slice } from 'prosemirror-model';
+import { ySyncPluginKey } from 'y-prosemirror';
 import { initTestEditor } from '@tests/helpers/helpers.js';
 
 /**
@@ -376,6 +378,38 @@ describe('StructuredContentLockPlugin', () => {
       // Assert: should handle gracefully (exact behavior depends on schema)
       expect(newState).toBeDefined();
     });
+
+    it('allows remote collaboration replacements that span locked SDTs', () => {
+      const doc = createDocWithSDT('sdtContentLocked', 'structuredContent');
+      const state = applyDocToEditor(doc);
+      const replacementParagraph = schema.nodes.paragraph.create(null, schema.text('Remote hello world'));
+      const replacementDoc = schema.nodes.doc.create(null, [replacementParagraph]);
+
+      const tr = state.tr
+        .replace(0, state.doc.content.size, new Slice(replacementDoc.content, 0, 0))
+        .setMeta(ySyncPluginKey, { isChangeOrigin: true });
+
+      const result = state.applyTransaction(tr);
+
+      expect(result.state.doc.textContent).toContain('Remote hello world');
+    });
+
+    it('allows snapshot-exit replacements that span locked SDTs (no isChangeOrigin)', () => {
+      const doc = createDocWithSDT('sdtContentLocked', 'structuredContent');
+      const state = applyDocToEditor(doc);
+      const replacementParagraph = schema.nodes.paragraph.create(null, schema.text('Snapshot exit'));
+      const replacementDoc = schema.nodes.doc.create(null, [replacementParagraph]);
+
+      // y-prosemirror's unrenderSnapshot() sets { snapshot: null, prevSnapshot: null }
+      // with no isChangeOrigin flag.
+      const tr = state.tr
+        .replace(0, state.doc.content.size, new Slice(replacementDoc.content, 0, 0))
+        .setMeta(ySyncPluginKey, { snapshot: null, prevSnapshot: null });
+
+      const result = state.applyTransaction(tr);
+
+      expect(result.state.doc.textContent).toContain('Snapshot exit');
+    });
   });
 
   describe('lock mode attribute validation', () => {