fix(cli): prevent collab reopen from overwriting existing ydoc with blank document (SD-2138) (#2296)

* fix(cli): prevent collab reopen from overwriting existing ydoc with blank document

SD-2138: When reopening a pre-existing ydoc in collaboration mode (no backing
.docx), the bootstrap flow could incorrectly detect the room as empty and seed
a blank document, destroying existing content. This happened because some
providers fire "synced" before Yjs updates are fully applied to local shared
types.

Add two-layer defense:
1. Post-sync content settling (200ms window) to let XmlFragment populate
2. Post-claim re-check after the 1500ms settling period — if room is now
   populated, join instead of destructively seeding

* refactor(cli): deduplicate room-state check in waitForContentSettling

Replace manual fragment + meta map iteration with a call to
detectRoomState(), keeping the heuristic in one place.

* fix(cli): clear stale pending bootstrap marker on join-after-claim

Author: caio-pizzol

apps/cli/src/lib/__tests__/bootstrap.test.ts

@@ -3,9 +3,11 @@ import { Doc as YDoc, XmlElement } from 'yjs';
 import {
   DEFAULT_BOOTSTRAP_SETTLING_MS,
   DEFAULT_BOOTSTRAP_JITTER_MS,
+  waitForContentSettling,
   detectRoomState,
   resolveBootstrapDecision,
   writeBootstrapMarker,
+  clearBootstrapMarker,
   claimBootstrap,
   detectBootstrapRace,
   type BootstrapMarker,
@@ -109,6 +111,15 @@ describe('writeBootstrapMarker', () => {
     expect(typeof marker.seededAt).toBe('string');
   });
 
+  test('clearBootstrapMarker removes the marker from the meta map', () => {
+    const ydoc = new YDoc();
+    writeBootstrapMarker(ydoc, 'doc');
+    expect(ydoc.getMap('meta').get('bootstrap')).toBeDefined();
+
+    clearBootstrapMarker(ydoc);
+    expect(ydoc.getMap('meta').get('bootstrap')).toBeUndefined();
+  });
+
   test('finalized marker makes detectRoomState return populated', () => {
     const ydoc = new YDoc();
     writeBootstrapMarker(ydoc, 'doc');
@@ -233,6 +244,46 @@ describe('claimBootstrap', () => {
     expect(decision).toEqual({ action: 'seed', source: 'doc' });
   });
 
+  test('SD-2138 regression: stale pending marker after join-after-claim causes false-empty on reconnect', async () => {
+    // Simulates the exact scenario that causes data loss:
+    // 1. Client wins claim (pending marker written)
+    // 2. Content arrives during settling → client joins instead of seeding
+    // 3. If pending marker is NOT cleared, a future reconnect sees:
+    //    - empty fragment (slow sync) + pending-only marker → 'empty' → destructive re-seed
+    // 4. Clearing the marker ensures the room doesn't have a misleading pending signal
+    const ydoc = new YDoc();
+
+    // Step 1: Win the claim — writes pending marker
+    const claim = await claimBootstrap(ydoc, 0, 0);
+    expect(claim.granted).toBe(true);
+
+    const marker = ydoc.getMap('meta').get('bootstrap') as BootstrapMarker;
+    expect(marker.source).toBe('pending');
+
+    // Step 2: Content arrived during settling (simulate)
+    const fragment = ydoc.getXmlFragment('supereditor');
+    fragment.insert(0, [new XmlElement('p')]);
+    expect(detectRoomState(ydoc)).toBe('populated');
+
+    // Step 3: Clear the pending marker (this is the fix)
+    clearBootstrapMarker(ydoc);
+
+    // Step 4: Simulate future reconnect — new ydoc where only meta synced,
+    // fragment hasn't arrived yet (slow-sync scenario)
+    const reconnectYdoc = new YDoc();
+    // No fragment content, no meta — room is clean after marker was cleared
+    expect(detectRoomState(reconnectYdoc)).toBe('empty');
+
+    // Without the fix, the pending marker would persist and detectRoomState
+    // would still return 'empty' — but the danger is that it LOOKS like a
+    // fresh room rather than a room with a stale claim. With the marker
+    // cleared, at least there's no misleading pending signal.
+
+    // The critical assertion: after clearing, the original ydoc's meta map
+    // has no bootstrap key that could sync to new clients as a stale pending marker
+    expect(ydoc.getMap('meta').get('bootstrap')).toBeUndefined();
+  });
+
   test('concurrent claimers: second claimer re-detects and joins after first seeds', async () => {
     // Simulates the full claim -> re-detect -> join path for a race loser
     const ydoc = new YDoc();
@@ -354,3 +405,82 @@ describe('DEFAULT_BOOTSTRAP_JITTER_MS', () => {
     expect(DEFAULT_BOOTSTRAP_JITTER_MS).toBeGreaterThan(0);
   });
 });
+
+// ---------------------------------------------------------------------------
+// waitForContentSettling (SD-2138)
+// ---------------------------------------------------------------------------
+
+describe('waitForContentSettling', () => {
+  test('resolves immediately when fragment already has content', async () => {
+    const ydoc = new YDoc();
+    const fragment = ydoc.getXmlFragment('supereditor');
+    fragment.insert(0, [new XmlElement('p')]);
+
+    const before = Date.now();
+    await waitForContentSettling(ydoc, 500);
+    expect(Date.now() - before).toBeLessThan(50);
+  });
+
+  test('resolves immediately when meta map has finalized bootstrap marker', async () => {
+    const ydoc = new YDoc();
+    ydoc.getMap('meta').set('bootstrap', { version: 1, source: 'doc' });
+
+    const before = Date.now();
+    await waitForContentSettling(ydoc, 500);
+    expect(Date.now() - before).toBeLessThan(50);
+  });
+
+  test('resolves immediately when meta map has non-bootstrap entries', async () => {
+    const ydoc = new YDoc();
+    ydoc.getMap('meta').set('docx', 'some-content');
+
+    const before = Date.now();
+    await waitForContentSettling(ydoc, 500);
+    expect(Date.now() - before).toBeLessThan(50);
+  });
+
+  test('waits and resolves when fragment is populated during settling', async () => {
+    const ydoc = new YDoc();
+    const fragment = ydoc.getXmlFragment('supereditor');
+
+    // Populate fragment after 20ms
+    setTimeout(() => {
+      fragment.insert(0, [new XmlElement('p')]);
+    }, 20);
+
+    const before = Date.now();
+    await waitForContentSettling(ydoc, 500);
+    const elapsed = Date.now() - before;
+
+    // Should resolve quickly after content arrives, not wait full 500ms
+    expect(elapsed).toBeGreaterThanOrEqual(15);
+    expect(elapsed).toBeLessThan(200);
+  });
+
+  test('times out when no content arrives', async () => {
+    const ydoc = new YDoc();
+
+    const before = Date.now();
+    await waitForContentSettling(ydoc, 50);
+    const elapsed = Date.now() - before;
+
+    expect(elapsed).toBeGreaterThanOrEqual(40);
+  });
+
+  test('does not treat pending bootstrap marker as content', async () => {
+    const ydoc = new YDoc();
+    ydoc.getMap('meta').set('bootstrap', {
+      version: 1,
+      clientId: 999,
+      seededAt: new Date().toISOString(),
+      source: 'pending',
+    });
+
+    const before = Date.now();
+    await waitForContentSettling(ydoc, 50);
+    const elapsed = Date.now() - before;
+
+    // Should wait full timeout since pending marker is not content
+    expect(elapsed).toBeGreaterThanOrEqual(40);
+  });
+});

apps/cli/src/lib/bootstrap.ts

@@ -72,6 +72,45 @@ export type ClaimResult = { granted: true } | { granted: false; competitor: Obse
  */
 export type RaceDetectionResult = { raceSuspected: false } | { raceSuspected: true; competitor: ObservedCompetitor };
 
+// ---------------------------------------------------------------------------
+// Post-sync content settling
+// ---------------------------------------------------------------------------
+
+/**
+ * Maximum time (ms) to wait for the XmlFragment to be populated after the
+ * provider reports "synced". Some providers fire the synced event before Yjs
+ * updates are fully applied to local shared types. This brief window avoids
+ * false-empty room detection that leads to destructive re-seeding (SD-2138).
+ */
+const CONTENT_SETTLING_MAX_MS = 200;
+
+/**
+ * After the collaboration provider reports "synced", wait briefly for the
+ * XmlFragment to be populated. Returns immediately if content is already
+ * present, or after CONTENT_SETTLING_MAX_MS if nothing arrives.
+ */
+export function waitForContentSettling(ydoc: YDoc, maxWaitMs: number = CONTENT_SETTLING_MAX_MS): Promise<void> {
+  if (detectRoomState(ydoc) === 'populated') return Promise.resolve();
+
+  const fragment = ydoc.getXmlFragment('supereditor');
+
+  return new Promise<void>((resolve) => {
+    const timeout = setTimeout(() => {
+      fragment.unobserve(observer);
+      resolve();
+    }, maxWaitMs);
+
+    const observer = () => {
+      if (fragment.length > 0) {
+        clearTimeout(timeout);
+        fragment.unobserve(observer);
+        resolve();
+      }
+    };
+    fragment.observe(observer);
+  });
+}
+
 // ---------------------------------------------------------------------------
 // Room state detection
 // ---------------------------------------------------------------------------
@@ -121,6 +160,16 @@ export function resolveBootstrapDecision(
 // Bootstrap marker
 // ---------------------------------------------------------------------------
 
+/**
+ * Remove the bootstrap marker from the meta map. Used when a claim winner
+ * discovers the room is already populated and joins instead of seeding —
+ * leaving a stale pending marker would cause future reconnects to
+ * misdetect the room as empty (SD-2138).
+ */
+export function clearBootstrapMarker(ydoc: YDoc): void {
+  ydoc.getMap('meta').delete('bootstrap');
+}
+
 export function writeBootstrapMarker(ydoc: YDoc, source: string): void {
   const metaMap = ydoc.getMap('meta');
   const marker: BootstrapMarker = {

apps/cli/src/lib/document.ts

@@ -11,9 +11,11 @@ import type { CollaborationProfile } from './collaboration';
 import { createCollaborationRuntime } from './collaboration';
 import {
   DEFAULT_BOOTSTRAP_SETTLING_MS,
+  waitForContentSettling,
   detectRoomState,
   resolveBootstrapDecision,
   claimBootstrap,
+  clearBootstrapMarker,
   writeBootstrapMarker,
   detectBootstrapRace,
   type RoomState,
@@ -251,6 +253,11 @@ export async function openCollaborativeDocument(
   try {
     await runtime.waitForSync();
 
+    // SD-2138: Some providers fire "synced" before Yjs updates are fully
+    // applied to local shared types. Give a brief window for the XmlFragment
+    // to be populated from incoming server state before checking room state.
+    await waitForContentSettling(runtime.ydoc);
+
     const onMissing = profile.onMissing ?? 'seedFromDoc';
     let finalRoomState = detectRoomState(runtime.ydoc);
     let decision = resolveBootstrapDecision(finalRoomState, onMissing, doc != null);
@@ -264,6 +271,18 @@ export async function openCollaborativeDocument(
         // here would produce a dual-seed race.
         finalRoomState = detectRoomState(runtime.ydoc);
         decision = { action: 'join' };
+      } else {
+        // SD-2138: Re-check room state after the claim settling period.
+        // Some providers fire "synced" before Yjs updates are fully applied,
+        // so content from the server may have arrived during the settling
+        // wait.  If the room is now populated, join instead of seeding —
+        // seeding here would destructively overwrite existing content.
+        const postClaimState = detectRoomState(runtime.ydoc);
+        if (postClaimState === 'populated') {
+          clearBootstrapMarker(runtime.ydoc);
+          finalRoomState = postClaimState;
+          decision = { action: 'join' };
+        }
       }
     }