fix(collaboration): harden part-sync hydration and dynamic OOXML key resolution

Author: harbournick

packages/super-editor/src/extensions/collaboration/part-sync/part-spec-registry.js

@@ -572,6 +572,58 @@ function deterministicDynamicId(partPath) {
   return `dyn_${encoded}`;
 }
 
+/**
+ * Decode a deterministic dynamic ID back to its part path.
+ *
+ * Inverse of deterministicDynamicId. Returns null for malformed encodings.
+ *
+ * @param {string} id
+ * @returns {string | null}
+ */
+function decodeDeterministicDynamicId(id) {
+  if (!id.startsWith('dyn_')) return null;
+  const encoded = id.slice('dyn_'.length);
+  let out = '';
+
+  for (let i = 0; i < encoded.length; ) {
+    const ch = encoded[i];
+    if (ch !== '_') {
+      out += ch;
+      i += 1;
+      continue;
+    }
+
+    const hex = encoded.slice(i + 1, i + 3);
+    if (!/^[0-9A-Fa-f]{2}$/.test(hex)) return null;
+
+    out += String.fromCharCode(parseInt(hex, 16));
+    i += 3;
+  }
+
+  return out;
+}
+
+/**
+ * Ensure a discovered dynamic spec is cached for this converter and indexed.
+ *
+ * @param {object | undefined} converter
+ * @param {PartSpec} spec
+ */
+function registerDiscoveredSpec(converter, spec) {
+  registerInPrefixIndex(spec);
+
+  if (!converter) return;
+  const cached = discoveredSpecsCache.get(converter);
+  if (cached) {
+    if (!cached.some((candidate) => candidate.id === spec.id)) {
+      cached.push(spec);
+    }
+    return;
+  }
+
+  discoveredSpecsCache.set(converter, [spec]);
+}
+
 /**
  * Scan a converter for XML parts not covered by static specs and create
  * generic PartSpecs for them. Results are cached per converter.
@@ -583,7 +635,14 @@ export function discoverGenericSpecs(converter) {
   if (!converter) return [];
 
   const cached = discoveredSpecsCache.get(converter);
-  if (cached) return cached;
+  if (cached) {
+    // Re-register cached specs because another converter invalidation can
+    // clear their shared prefixIndex entries.
+    for (const spec of cached) {
+      registerInPrefixIndex(spec);
+    }
+    return cached;
+  }
 
   const store = converter.parts ?? converter.convertedXml ?? {};
   const specs = [];
@@ -685,6 +744,19 @@ export function resolveOoxmlPartKey(key, converter) {
     if (section != null) return { spec, section };
   }
 
+  // Infer dynamic spec directly from incoming key so late-joining clients can
+  // resolve remote dyn_* keys that are not in local converter.parts yet.
+  if (slashIdx !== -1) {
+    const id = key.slice(0, slashIdx);
+    const partPath = decodeDeterministicDynamicId(id);
+    if (partPath && !isExcludedFromDiscovery(partPath)) {
+      const spec = createXmlPartSpec(id, partPath);
+      registerDiscoveredSpec(converter, spec);
+      const section = spec.parseKey(key);
+      if (section != null) return { spec, section };
+    }
+  }
+
   return null;
 }
 
@@ -705,9 +777,19 @@ export function resolveOoxmlPartKey(key, converter) {
  */
 export function resolvePartChangedSpec(partId, changedPaths, converter) {
   if (partId === 'styles') {
+    const sectionHints = changedPaths
+      ?.map((path) => {
+        if (typeof path !== 'string') return null;
+        const noArrayIndices = path.replace(/\[\d+\]/g, '');
+        const section = noArrayIndices.split('.')[0];
+        return STYLE_SECTIONS.includes(section) ? section : null;
+      })
+      .filter(Boolean);
+
+    const dedupedSectionHints = sectionHints?.length > 0 ? [...new Set(sectionHints)] : undefined;
     return {
       spec: STYLES_SPEC,
-      sectionHints: changedPaths?.map((p) => p.split('.')[0]),
+      sectionHints: dedupedSectionHints,
     };
   }
 

packages/super-editor/src/extensions/collaboration/part-sync/part-spec-registry.test.js

@@ -759,6 +759,21 @@ describe('resolveOoxmlPartKey — prefix index', () => {
     expect(result).not.toBeNull();
     expect(result.section).toBe('root');
   });
+
+  it('resolves dyn_* keys from incoming key even when part is not local yet', () => {
+    const converter = { parts: {} };
+    const result = resolveOoxmlPartKey('dyn_customXml_2FremoteOnly_2Exml/root', converter);
+    expect(result).not.toBeNull();
+    expect(result.spec.id).toBe('dyn_customXml_2FremoteOnly_2Exml');
+    expect(result.spec.partPath).toBe('customXml/remoteOnly.xml');
+    expect(result.section).toBe('root');
+  });
+
+  it('rejects inferred dyn_* keys that decode to excluded/static-covered paths', () => {
+    const converter = { parts: {} };
+    const result = resolveOoxmlPartKey('dyn_word_2Fdocument_2Exml/root', converter);
+    expect(result).toBeNull();
+  });
 });
 
 // ---------------------------------------------------------------------------
@@ -772,6 +787,18 @@ describe('resolvePartChangedSpec', () => {
     expect(result.sectionHints).toEqual(['docDefaults', 'styles']);
   });
 
+  it('normalizes styles array-index changed paths to valid style sections', () => {
+    const result = resolvePartChangedSpec('styles', ['styles[0].name', 'styles[10].runProperties.bold']);
+    expect(result.spec).toBe(STYLES_SPEC);
+    expect(result.sectionHints).toEqual(['styles']);
+  });
+
+  it('falls back to full styles publish when changedPaths do not map to known sections', () => {
+    const result = resolvePartChangedSpec('styles', ['unknownBranch.value']);
+    expect(result.spec).toBe(STYLES_SPEC);
+    expect(result.sectionHints).toBeUndefined();
+  });
+
   it('routes header:rId1 to HEADER_FOOTER_CONTENT_SPEC', () => {
     const result = resolvePartChangedSpec('header:rId1');
     expect(result.spec).toBe(HEADER_FOOTER_CONTENT_SPEC);
@@ -918,6 +945,25 @@ describe('discoverGenericSpecs', () => {
     const specs2 = discoverGenericSpecs(converter2);
     expect(specs1[0].id).toBe(specs2[0].id);
   });
+
+  it('re-registers cached dynamic prefixes after another converter invalidates', () => {
+    const converter1 = {
+      parts: { 'customXml/item1.xml': { elements: [{ name: 'root' }] } },
+    };
+    const converter2 = {
+      parts: { 'customXml/item1.xml': { elements: [{ name: 'root' }] } },
+    };
+
+    discoverGenericSpecs(converter1);
+    discoverGenericSpecs(converter2);
+
+    // This removes dyn_customXml_2Fitem1_2Exml/ from the shared prefix index.
+    invalidateDiscoveredSpecs(converter1);
+
+    const result = resolveOoxmlPartKey('dyn_customXml_2Fitem1_2Exml/root', converter2);
+    expect(result).not.toBeNull();
+    expect(result.section).toBe('root');
+  });
 });
 
 describe('invalidateDiscoveredSpecs', () => {

packages/super-editor/src/extensions/collaboration/part-sync/part-sync-engine.js

@@ -16,6 +16,48 @@
 
 import { EXCLUDED_PART_PATHS } from './part-spec-registry.js';
 
+const SPEC_READY_META_PREFIX = '__specReady/';
+
+/**
+ * Build a stable metadata key for per-spec readiness tracking.
+ *
+ * @param {import('./part-spec-registry.js').PartSpec} spec
+ * @returns {string}
+ */
+function getSpecReadinessMetaKey(spec) {
+  return `${SPEC_READY_META_PREFIX}${spec.channel}:${spec.id}`;
+}
+
+/**
+ * Determine whether remote has ever published this spec.
+ *
+ * Uses an explicit readiness marker plus a legacy fallback that scans section
+ * metadata keys, so older rooms still hydrate/delete correctly.
+ *
+ * @param {object} metaMap
+ * @param {import('./part-spec-registry.js').PartSpec} spec
+ * @returns {boolean}
+ */
+function hasRemoteSpecReadiness(metaMap, spec) {
+  if (!metaMap) return false;
+
+  const readinessKey = getSpecReadinessMetaKey(spec);
+  if (typeof metaMap.has === 'function' && metaMap.has(readinessKey)) {
+    return true;
+  }
+
+  if (typeof metaMap.forEach !== 'function') return false;
+
+  let found = false;
+  metaMap.forEach((_value, key) => {
+    if (found || typeof key !== 'string') return;
+    if (spec.parseKey(key) != null) {
+      found = true;
+    }
+  });
+  return found;
+}
+
 // ---------------------------------------------------------------------------
 // Per-editor guard state
 // ---------------------------------------------------------------------------
@@ -122,6 +164,8 @@ export function publishPartSections(editor, spec, sectionHints) {
 
   const userId = editor.options.user?.id ?? 'unknown';
   const metaMap = ydoc.getMap('ooxmlPartMeta');
+  const now = Date.now();
+  const readinessKey = getSpecReadinessMetaKey(spec);
 
   ydoc.transact(
     () => {
@@ -135,8 +179,12 @@ export function publishPartSections(editor, spec, sectionHints) {
         map.set('_version', spec.version);
       }
       for (const { key } of writes) {
-        metaMap.set(key, { updatedBy: userId, updatedAt: Date.now() });
+        metaMap.set(key, { updatedBy: userId, updatedAt: now });
       }
+      // Marks this spec as remotely initialized even if all section keys were
+      // later deleted. Hydration uses this to decide whether empty remote
+      // state should delete stale local sections or trigger first-time seeding.
+      metaMap.set(readinessKey, { updatedBy: userId, updatedAt: now, version: spec.version ?? null });
     },
     { event: `${spec.id}-publish`, user: editor.options.user },
   );
@@ -264,6 +312,7 @@ export function hydrateOrSeedPart(editor, spec) {
   if (!ydoc || ydoc.isDestroyed) return;
 
   const map = ydoc.getMap(spec.channel);
+  const metaMap = ydoc.getMap('ooxmlPartMeta');
 
   if (map.has('_version')) {
     // Room has this channel — hydrate from remote authority.
@@ -279,20 +328,27 @@ export function hydrateOrSeedPart(editor, spec) {
       });
     }
 
-    if (keys.length > 0) {
-      applyRemotePartSections(editor, spec, map, keys);
-    }
+    const hasRemoteSpecState = keys.length > 0 || hasRemoteSpecReadiness(metaMap, spec);
+    if (hasRemoteSpecState) {
+      if (keys.length > 0) {
+        applyRemotePartSections(editor, spec, map, keys);
+      }
 
-    // Remove local sections that no longer exist remotely. This handles parts
-    // deleted in-room before this client joined — the Y.Map has _version but
-    // no remaining keys for this spec, yet bootstrap left stale local data.
-    if (editor.converter && spec.removeSection) {
-      const localSections = spec.listSections(editor.converter);
-      const remoteSet = new Set(keys.map((k) => spec.parseKey(k)));
-      const staleKeys = localSections.filter((s) => !remoteSet.has(s)).map((s) => spec.sectionKey(s));
-      if (staleKeys.length > 0) {
-        deleteRemotePartSections(editor, spec, staleKeys);
+      // Remove local sections that no longer exist remotely, but only once this
+      // specific spec has remote readiness. Shared channels (ooxmlPartModels)
+      // can have _version set by another spec before this one is initialized.
+      if (editor.converter && spec.removeSection) {
+        const localSections = spec.listSections(editor.converter);
+        const remoteSet = new Set(keys.map((k) => spec.parseKey(k)));
+        const staleKeys = localSections.filter((s) => !remoteSet.has(s)).map((s) => spec.sectionKey(s));
+        if (staleKeys.length > 0) {
+          deleteRemotePartSections(editor, spec, staleKeys);
+        }
       }
+    } else if (editor.converter) {
+      // Channel exists but this spec has never been initialized remotely.
+      // Seed local state instead of deleting it as stale.
+      publishPartSections(editor, spec);
     }
   } else if (editor.converter) {
     // Room doesn't have this channel — seed from local converter

packages/super-editor/src/extensions/collaboration/part-sync/part-sync-engine.test.js

@@ -441,8 +441,9 @@ describe('hydrateOrSeedPart', () => {
     expect(spec.applySection).not.toHaveBeenCalled();
   });
 
-  it('removes stale local sections that were deleted remotely', () => {
-    // Room has _version but no keys for this spec → all local sections are stale
+  it('seeds local sections when channel has _version but spec is not remotely initialized', () => {
+    // Shared channel may have _version set by another spec while this one is
+    // still uninitialized remotely. In that case we seed instead of delete.
     const channelMap = createMockYMap({ _version: 1 });
     const ydoc = createMockYdoc({ testChannel: channelMap });
     editor = createMockEditor({ options: { ydoc, user: { id: 'user-1' } } });
@@ -453,7 +454,26 @@ describe('hydrateOrSeedPart', () => {
 
     hydrateOrSeedPart(editor, spec);
 
-    // Should not apply (nothing remote), but should remove stale local 'root'
+    // Should seed local root to remote channel and not delete local state.
+    expect(spec.applySection).not.toHaveBeenCalled();
+    expect(channelMap.set).toHaveBeenCalledWith('test-spec/root', expect.anything());
+    expect(spec.removeSection).not.toHaveBeenCalled();
+  });
+
+  it('removes stale local sections when spec has remote readiness metadata', () => {
+    const channelMap = createMockYMap({ _version: 1 });
+    const metaMap = createMockYMap({
+      'test-spec/root': { updatedBy: 'user-2', updatedAt: 1000 },
+    });
+    const ydoc = createMockYdoc({ testChannel: channelMap, ooxmlPartMeta: metaMap });
+    editor = createMockEditor({ options: { ydoc, user: { id: 'user-1' } } });
+    spec = createMockSpec({
+      listSections: vi.fn(() => ['root']),
+      removeSection: vi.fn(),
+    });
+
+    hydrateOrSeedPart(editor, spec);
+
     expect(spec.applySection).not.toHaveBeenCalled();
     expect(spec.removeSection).toHaveBeenCalledWith(editor.converter, 'root');
   });