Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
130 changes: 130 additions & 0 deletions apps/cli/src/lib/__tests__/bootstrap.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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);
});
});
49 changes: 49 additions & 0 deletions apps/cli/src/lib/bootstrap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
// ---------------------------------------------------------------------------
Expand Down Expand Up @@ -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 = {
Expand Down
19 changes: 19 additions & 0 deletions apps/cli/src/lib/document.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand All @@ -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' };
}
Comment thread
harbournick marked this conversation as resolved.
}
}

Expand Down
Loading
Loading