chore: replace blind sleep-and-check bootstrap with reactive observe-based claim detection

Author: harbournick

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

@@ -2,13 +2,19 @@ import { describe, expect, test } from 'bun:test';
 import { Doc as YDoc, XmlElement } from 'yjs';
 import {
   DEFAULT_BOOTSTRAP_SETTLING_MS,
+  DEFAULT_BOOTSTRAP_JITTER_MS,
   detectRoomState,
   resolveBootstrapDecision,
   writeBootstrapMarker,
   claimBootstrap,
+  detectBootstrapRace,
   type BootstrapMarker,
 } from '../bootstrap';
 
+// ---------------------------------------------------------------------------
+// detectRoomState
+// ---------------------------------------------------------------------------
+
 describe('detectRoomState', () => {
   test('returns "empty" for a fresh ydoc', () => {
     const ydoc = new YDoc();
@@ -54,6 +60,10 @@ describe('detectRoomState', () => {
   });
 });
 
+// ---------------------------------------------------------------------------
+// resolveBootstrapDecision
+// ---------------------------------------------------------------------------
+
 describe('resolveBootstrapDecision', () => {
   test('populated room always joins', () => {
     expect(resolveBootstrapDecision('populated', 'seedFromDoc', true)).toEqual({ action: 'join' });
@@ -62,26 +72,30 @@ describe('resolveBootstrapDecision', () => {
     expect(resolveBootstrapDecision('populated', 'error', true)).toEqual({ action: 'join' });
   });
 
-  test('empty + seedFromDoc + hasDoc → seed from doc', () => {
+  test('empty + seedFromDoc + hasDoc -> seed from doc', () => {
     expect(resolveBootstrapDecision('empty', 'seedFromDoc', true)).toEqual({ action: 'seed', source: 'doc' });
   });
 
-  test('empty + seedFromDoc + no doc → seed from blank', () => {
+  test('empty + seedFromDoc + no doc -> seed from blank', () => {
     expect(resolveBootstrapDecision('empty', 'seedFromDoc', false)).toEqual({ action: 'seed', source: 'blank' });
   });
 
-  test('empty + blank → seed from blank regardless of hasDoc', () => {
+  test('empty + blank -> seed from blank regardless of hasDoc', () => {
     expect(resolveBootstrapDecision('empty', 'blank', true)).toEqual({ action: 'seed', source: 'blank' });
     expect(resolveBootstrapDecision('empty', 'blank', false)).toEqual({ action: 'seed', source: 'blank' });
   });
 
-  test('empty + error → error', () => {
+  test('empty + error -> error', () => {
     const result = resolveBootstrapDecision('empty', 'error', true);
     expect(result.action).toBe('error');
     expect((result as { reason: string }).reason).toContain('onMissing');
   });
 });
 
+// ---------------------------------------------------------------------------
+// writeBootstrapMarker
+// ---------------------------------------------------------------------------
+
 describe('writeBootstrapMarker', () => {
   test('writes marker to meta map with correct shape', () => {
     const ydoc = new YDoc();
@@ -102,30 +116,34 @@ describe('writeBootstrapMarker', () => {
   });
 });
 
+// ---------------------------------------------------------------------------
+// claimBootstrap
+// ---------------------------------------------------------------------------
+
 describe('claimBootstrap', () => {
-  test('returns true when this client owns the marker', async () => {
+  test('returns granted when this client owns the marker', async () => {
     const ydoc = new YDoc();
-    const result = await claimBootstrap(ydoc, 0);
-    expect(result).toBe(true);
+    const result = await claimBootstrap(ydoc, 0, 0);
+    expect(result.granted).toBe(true);
 
     const marker = ydoc.getMap('meta').get('bootstrap') as BootstrapMarker;
     expect(marker.clientId).toBe(ydoc.clientID);
   });
 
   test('claim marker has source "pending"', async () => {
     const ydoc = new YDoc();
-    await claimBootstrap(ydoc, 0);
+    await claimBootstrap(ydoc, 0, 0);
 
     const marker = ydoc.getMap('meta').get('bootstrap') as BootstrapMarker;
     expect(marker.source).toBe('pending');
   });
 
-  test('returns false when another client overwrites the marker during settling', async () => {
+  test('returns denied with competitor info when another client overwrites during settling', async () => {
     const ydoc = new YDoc();
     const otherClientId = ydoc.clientID + 1;
     const metaMap = ydoc.getMap('meta');
 
-    const promise = claimBootstrap(ydoc, 10);
+    const promise = claimBootstrap(ydoc, 20, 0);
 
     // Overwrite with the other client's marker during the settling window
     setTimeout(() => {
@@ -138,11 +156,67 @@ describe('claimBootstrap', () => {
     }, 2);
 
     const result = await promise;
-    expect(result).toBe(false);
+    expect(result.granted).toBe(false);
+    if (!result.granted) {
+      expect(result.competitor.observedOtherClientId).toBe(otherClientId);
+      expect(result.competitor.observedSource).toBe('pending');
+      expect(typeof result.competitor.observedAt).toBe('string');
+    }
+  });
+
+  test('observe detects late-arriving marker after sleep ends', async () => {
+    // Simulates network latency: the competing marker arrives just before
+    // the final read, but the observe handler catches it reactively.
+    const ydoc = new YDoc();
+    const otherClientId = ydoc.clientID + 1;
+    const metaMap = ydoc.getMap('meta');
+
+    const promise = claimBootstrap(ydoc, 5, 0);
+
+    // Overwrite at ~4ms — very close to when the sleep ends
+    setTimeout(() => {
+      metaMap.set('bootstrap', {
+        version: 1,
+        clientId: otherClientId,
+        seededAt: new Date().toISOString(),
+        source: 'pending',
+      });
+    }, 4);
+
+    const result = await promise;
+    expect(result.granted).toBe(false);
+  });
+
+  test('returns denied gracefully when marker is removed during settling', async () => {
+    const ydoc = new YDoc();
+    const metaMap = ydoc.getMap('meta');
+
+    const promise = claimBootstrap(ydoc, 20, 0);
+
+    // Another process deletes the bootstrap key during settling
+    setTimeout(() => {
+      metaMap.delete('bootstrap');
+    }, 2);
+
+    const result = await promise;
+    expect(result.granted).toBe(false);
+    if (!result.granted) {
+      expect(result.competitor.observedOtherClientId).toBe(0);
+      expect(result.competitor.observedSource).toBe('unknown');
+    }
+  });
+
+  test('jitter=0 disables random delay', async () => {
+    const ydoc = new YDoc();
+    const before = Date.now();
+    await claimBootstrap(ydoc, 0, 0);
+    const elapsed = Date.now() - before;
+    // With jitter=0 and settling=0, should complete almost instantly
+    expect(elapsed).toBeLessThan(50);
   });
 
   test('stale pending marker does not block subsequent bootstrap detection', async () => {
-    // Simulates Finding 1: claimer crashes after writing pending marker
+    // Simulates: claimer crashes after writing pending marker
     const ydoc = new YDoc();
     ydoc.getMap('meta').set('bootstrap', {
       version: 1,
@@ -160,7 +234,7 @@ describe('claimBootstrap', () => {
   });
 
   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
+    // Simulates the full claim -> re-detect -> join path for a race loser
     const ydoc = new YDoc();
     const otherClientId = ydoc.clientID + 1;
     const metaMap = ydoc.getMap('meta');
@@ -180,12 +254,16 @@ describe('claimBootstrap', () => {
   });
 });
 
+// ---------------------------------------------------------------------------
+// claim loser always yields
+// ---------------------------------------------------------------------------
+
 describe('claim loser always yields', () => {
   test('loser yields even when winner marker is still pending (room looks empty)', () => {
     // After a failed claim, the loser sees the room with only a pending
     // marker (the winner hasn't finalized yet).  detectRoomState returns
     // 'empty' but the loser must NOT re-seed — they must yield.
-    // This tests the contract that document.ts enforces: claim loser → join.
+    // This tests the contract that document.ts enforces: claim loser -> join.
     const ydoc = new YDoc();
     ydoc.getMap('meta').set('bootstrap', {
       version: 1,
@@ -206,8 +284,73 @@ describe('claim loser always yields', () => {
   });
 });
 
+// ---------------------------------------------------------------------------
+// detectBootstrapRace
+// ---------------------------------------------------------------------------
+
+describe('detectBootstrapRace', () => {
+  test('returns raceSuspected: false when no competing marker arrives', async () => {
+    const ydoc = new YDoc();
+    writeBootstrapMarker(ydoc, 'doc');
+
+    const result = await detectBootstrapRace(ydoc, 10);
+    expect(result.raceSuspected).toBe(false);
+  });
+
+  test('returns raceSuspected: true with competitor info when another finalized marker arrives', async () => {
+    const ydoc = new YDoc();
+    const otherClientId = ydoc.clientID + 1;
+    writeBootstrapMarker(ydoc, 'doc');
+
+    const promise = detectBootstrapRace(ydoc, 20);
+
+    // Another client's finalized marker arrives during observation
+    setTimeout(() => {
+      ydoc.getMap('meta').set('bootstrap', {
+        version: 1,
+        clientId: otherClientId,
+        seededAt: new Date().toISOString(),
+        source: 'doc',
+      });
+    }, 5);
+
+    const result = await promise;
+    expect(result.raceSuspected).toBe(true);
+    if (result.raceSuspected) {
+      expect(result.competitor.observedOtherClientId).toBe(otherClientId);
+      expect(result.competitor.observedSource).toBe('doc');
+      expect(typeof result.competitor.observedAt).toBe('string');
+    }
+  });
+
+  test('ignores changes to non-bootstrap meta keys', async () => {
+    const ydoc = new YDoc();
+    writeBootstrapMarker(ydoc, 'doc');
+
+    const promise = detectBootstrapRace(ydoc, 20);
+
+    // Unrelated meta key changes should not trigger false positive
+    setTimeout(() => {
+      ydoc.getMap('meta').set('docx', 'some-content');
+    }, 5);
+
+    const result = await promise;
+    expect(result.raceSuspected).toBe(false);
+  });
+});
+
+// ---------------------------------------------------------------------------
+// Constants
+// ---------------------------------------------------------------------------
+
 describe('DEFAULT_BOOTSTRAP_SETTLING_MS', () => {
   test('is a positive number', () => {
     expect(DEFAULT_BOOTSTRAP_SETTLING_MS).toBeGreaterThan(0);
   });
 });
+
+describe('DEFAULT_BOOTSTRAP_JITTER_MS', () => {
+  test('is a positive number', () => {
+    expect(DEFAULT_BOOTSTRAP_JITTER_MS).toBeGreaterThan(0);
+  });
+});