[pull] main from liveblocks:main

CHANGELOG.md

@@ -1,6 +1,12 @@
 ## vNEXT (not yet released)
 
-- Export internal utility
+## v3.19.5
+
+### `@liveblocks/client`
+
+- Fix a `LiveList` divergence after reconnects: a pending `push` could under
+  specific timing conditions during a reconnect still cause a divergence between
+  clients, despite the fix from 3.19.4.
 
 ## v3.19.4
 

packages/liveblocks-chat-sdk-adapter/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@liveblocks/chat-sdk-adapter",
-  "version": "3.19.4",
+  "version": "3.19.5",
   "description": "Liveblocks adapter for the Chat SDK.",
   "license": "Apache-2.0",
   "author": "Liveblocks Inc.",

packages/liveblocks-client/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@liveblocks/client",
-  "version": "3.19.4",
+  "version": "3.19.5",
   "description": "A client that lets you interact with Liveblocks servers. Liveblocks is the all-in-one toolkit to build collaborative products like Figma, Notion, and more.",
   "license": "Apache-2.0",
   "author": "Liveblocks Inc.",

packages/liveblocks-core/e2e/README.md

@@ -44,6 +44,6 @@ Run a specific test file:
 npx turbo test:e2e -- e2e/list-insert.test.ts
 ```
 
-**Note**: Since these tests run against an actual production deployment, they
-require a `LIVEBLOCKS_PUBLIC_KEY` environment variable to connect to the
-Liveblocks service.
+**Note**: These tests run against a local Liveblocks dev server, which the
+`test:e2e` script starts automatically (`liveblocks dev`). No API key is
+needed; set `LIVEBLOCKS_DEV_SERVER_PORT` to override the default port (1154).

packages/liveblocks-core/e2e/list-consistency.test.ts

@@ -91,7 +91,7 @@ test(
     // This test verifies that undo/redo operations maintain consistency across clients
     // when operations are performed on different clients in a distributed environment.
     async ({ root1, root2, room1, room2, control, assert }) => {
-      // Client A does a move operation: move C (index 2) to position 0
+      // Client A moves 🟢 (index 2) to position 0; Client B deletes it
       root1.get("list").move(2, 0);
       root2.get("list").delete(2);
       assert(

packages/liveblocks-core/e2e/list-push-reconnect-divergence.test.ts

@@ -1,7 +1,8 @@
 import { expect, test } from "vitest";
 
 import { LiveList } from "../src/crdts/LiveList";
-import { prepareTestsConflicts } from "./utils";
+import { withTimeout } from "../src/lib/utils";
+import { prepareTestsConflicts, waitUntilStatus } from "./utils";
 
 const sleep = (ms: number) => new Promise((r) => setTimeout(r, ms));
 
@@ -21,16 +22,15 @@ async function waitUntil(
 }
 
 /**
- * Deterministic reproduction of the offline.test.ts "client synchronizes
- * offline changes" divergence.
+ * Deterministic version of the offline.test.ts "client synchronizes offline
+ * changes" scenario.
  *
- * The trigger is an item that the server has already stored but that the
+ * The tricky case is an item that the server has already stored but that the
  * pushing client still considers pending (unacknowledged), because the client
- * never received the ack. On reconnect, the client's optimistic tail-bump
- * moves that pending push past a sibling the other client added, re-sends it at
- * its original key, and the server bare-acks it (already stored, no
- * reposition), so the bump is never undone. The two clients then disagree on
- * the order.
+ * never received the ack. On reconnect, the snapshot also carries a sibling
+ * the other client appended in the meantime. The still-pending item must keep
+ * its server position (before the sibling) on both clients, rather than being
+ * optimistically bumped past it by its own re-sent push.
  */
 test(
   "a pending push the server already stored keeps its server position after reconnect",
@@ -40,11 +40,12 @@ test(
       const list1 = root1.get("list");
       const list2 = root2.get("list");
 
-      // 1. Client A pushes P and flushes it to the server, but drops every
-      //    incoming message first, so the server's ack/echo never reaches A: P
-      //    is stored server-side (so B sees it) yet stays *pending* on A.
+      // 1. Client A pushes P and flushes it to the server, but stalls its
+      //    downlink first, so the server's ack/echo never reaches A before the
+      //    connection drops: P is stored server-side (so B sees it) yet stays
+      //    *pending* on A.
       list1.push("P");
-      control.dropIncomingA();
+      control.pauseIncomingA();
       control.flushSyncA();
       await waitUntil(
         () => [...list2].includes("P"),
@@ -62,9 +63,9 @@ test(
         "Client B sees [P, Q]"
       );
 
-      // 4. A reconnects. The snapshot carries both P and Q; A's tail-bump moves
-      //    its still-pending P past Q, then re-sends P at its original key. The
-      //    server bare-acks (P already there), so A's bump is never undone.
+      // 4. A reconnects. The snapshot carries both P and Q, and A re-sends its
+      //    still-pending P. P is already stored server-side, so it must keep
+      //    its server position (before Q) on both clients.
       room1.reconnect();
 
       await waitUntil(
@@ -76,8 +77,76 @@ test(
       await sleep(500);
 
       // Both clients must agree on the server's order, [P, Q].
-      expect([...list1]).toEqual([...list2]);
-      expect([...list2]).toEqual(["P", "Q"]);
+      expect(list1.toJSON()).toEqual(list2.toJSON());
+      expect(list2.toJSON()).toEqual(["P", "Q"]);
+    }
+  )
+);
+
+/**
+ * Same divergence, but reached *after* the snapshot reconcile, via a live op.
+ *
+ * After a reconnect, the snapshot reconcile itself adopts the server's
+ * positions, but the re-sent pending push stays unacknowledged until the
+ * server's ack lands. A remote sibling push arriving as a live op inside that
+ * window triggers the optimistic tail-bump, which moves the pending push past
+ * the sibling. The server already stored the push, so the re-send is acked
+ * without a repositioning op, and the bump is never undone.
+ *
+ * In the wild this window is widened by large list items (their re-send is
+ * slow to reach the server), which is why the bug shows up intermittently and
+ * mostly with big payloads. The test simulates that slowness by stalling A's
+ * uplink while the re-send sits on it.
+ */
+test(
+  "a sibling pushed while a re-sent pending push awaits its ack keeps its server position",
+  prepareTestsConflicts(
+    { list: new LiveList<string>([]) },
+    async ({ root1, root2, room1, control }) => {
+      const list1 = root1.get("list");
+      const list2 = root2.get("list");
+
+      // 1. Client A pushes P and flushes it to the server, but stalls its
+      //    downlink first, so the server's ack/echo never reaches A before the
+      //    connection drops: P is stored server-side (so B sees it) yet stays
+      //    *pending* on A.
+      list1.push("P");
+      control.pauseIncomingA();
+      await control.flushA(); // Ensure client B sees P
+
+      // 2. A reconnects, and we stall the fresh socket's uplink right after it
+      //    connects: at that point its FETCH_STORAGE request is already out
+      //    (sent synchronously on connect), but the snapshot needs a server
+      //    round trip, so the reconcile hasn't run yet. The reconcile then
+      //    puts the re-send of P on the stalled uplink instead of on the wire,
+      //    keeping P pending. The reconcile signals completion through the
+      //    storageDidLoad event.
+      const reconciled$ = room1.events.storageDidLoad.waitUntil();
+      room1.reconnect();
+      await waitUntilStatus(room1, "connecting");
+      await waitUntilStatus(room1, "connected");
+      control.pauseA();
+      await withTimeout(
+        reconciled$,
+        10_000,
+        "Client A did not reconcile after reconnect within 10s"
+      );
+
+      // 3. B pushes Q. It reaches A as a live op while P is still pending.
+      list2.push("Q");
+      await control.flushB();
+
+      // 4. Only now release P's re-send. P is already stored server-side, so
+      //    it must keep its server position (before Q) on both clients.
+      await control.flushA();
+
+      // Let any acks settle. (flushA's beacon is confirmed by Client B, so it
+      // says nothing about A having received its ack yet.)
+      await sleep(500);
+
+      // Both clients must agree on the server's order, [P, Q].
+      expect(list2.toJSON()).toEqual(["P", "Q"]);
+      expect(list1.toJSON()).toEqual(list2.toJSON());
     }
   )
 );

packages/liveblocks-core/e2e/list-push.test.ts

@@ -4,11 +4,11 @@ import { LiveList } from "../src/crdts/LiveList";
 import { prepareTestsConflicts } from "./utils";
 
 // Two actors append to the same LiveList near-simultaneously: client A appends
-// a1 then a2; client B appends b1 without yet having seen a1/a2, so b1 guesses
-// the head position. By the time b1 reaches the server, a1 and a2 are already
-// stored, and the position conflict is resolved *between* them — so the list
-// settles as [a1, b1, a2] instead of append order [a1, a2, b1].
-// A server-authoritative append must place b1 at the true end.
+// a1 then a2; client B appends b1 without yet having seen a1/a2, so b1's
+// client-computed position is stale by the time it reaches the server (a1 and
+// a2 are already stored there). Because the op is tagged with intent: "push",
+// the server ignores that stale position and appends b1 at the true end, so
+// both clients settle in append order: [a1, a2, b1].
 test(
   "concurrent pushes settle in append order, never wedged",
   prepareTestsConflicts(

packages/liveblocks-core/e2e/list-set.test.ts

@@ -215,7 +215,7 @@ test(
       list: new LiveList(["a"]),
     },
     async ({ root1, root2, control, assert }) => {
-      // Client A replaces "a" with "X"
+      // Client A replaces "a" with "🟢"
       root1.get("list").set(0, "🟢");
 
       // Client B simultaneously deletes "a"
@@ -245,8 +245,6 @@ test(
     },
     async ({ root1, root2, control, assert }) => {
       // Client A changes "a" to "🟢" and moves it after "b"
-      // This is done in a batch to ensure the default throttling won't
-      // send the second operation in the message queue
       root1.get("list").set(0, "🟢");
       root1.get("list").move(0, 1);
       assert(

packages/liveblocks-core/e2e/storage-notification-reconnect.test.ts

@@ -17,9 +17,10 @@
  *
  * NOTE ON CONTROL KEYS: several LiveObject tests carry an unchanged scalar key
  * (e.g. `keep`) that the mutation never touches. The reconnect path routes a
- * snapshot through `getTreesDiffOperations`, which re-sends the *full*
- * UPDATE_OBJECT data — so an unchanged key can be spuriously re-notified. The
- * control key is what makes that bug observable; do not remove it.
+ * snapshot through `getTreesDiffOperations`, whose UPDATE_OBJECT ops must
+ * carry only the keys that actually changed — a full-data re-send would
+ * spuriously re-notify the unchanged key. The control key is what makes that
+ * observable; do not remove it.
  */
 import { expect, onTestFinished, test } from "vitest";
 import WebSocket from "ws";
@@ -483,11 +484,10 @@ test("LiveObject: nested-object deletes fire equivalent notifications online and
   ).toEqual({ x: 1 });
 });
 
-// Baseline (passes today): when the transitioned key is the object's *only*
-// scalar, moving it into a child node empties the object's `data`, so the
-// snapshot diff produces an UPDATE_OBJECT with empty data — nothing left for
-// the full-data re-send to spuriously re-notify. Contrast with the next test,
-// which adds a surviving scalar sibling and exposes that exact leak.
+// Baseline: when the transitioned key is the object's *only* scalar, moving
+// it into a child node empties the object's `data`, so the snapshot diff has
+// no other scalar keys to consider. The next test adds a surviving scalar
+// sibling, which the diff must not spuriously re-notify.
 test("LiveObject: scalar→nested-object transition (sole key) fires equivalent notifications online and on reconnect", async () => {
   const { online, reconnect } = await bothPhases(
     () => ({
@@ -573,9 +573,8 @@ test("LiveObject: nested-object→scalar transition fires equivalent notificatio
 //   - the online path saw the intermediate churn while the reconnect path saw
 //     only the collapsed net result.
 //
-// Unlike the bug-spec tests above, these are expected to PASS on the current
-// path — they lock in the collapse semantics so the reconcile refactor can't
-// regress them.
+// These lock in the collapse semantics so the reconcile refactor can't regress
+// them.
 // ─────────────────────────────────────────────────────────────────────────────
 
 const insertedItems = (deltas: ListUpdate["updates"]): unknown[] =>