Merge pull request 'feat(tree): server-authoritative realtime tree updates' (#15) from feat/realtime-tree-server into develop

apps/client/src/features/page/queries/page-query.ts

@@ -360,6 +360,16 @@ export function invalidateOnCreatePage(data: Partial<IPage>) {
     queryKey,
     (old) => {
       if (!old) return old;
+
+      // Idempotency guard: the server now self-echoes addTreeNode back to the
+      // author, so this writer can run twice for one create (mutation onSuccess
+      // + socket echo). Skip the append if the page is already in the cache to
+      // avoid a duplicate node / duplicate React key.
+      const exists = old.pages.some((page) =>
+        page.items.some((item) => item.id === newPage.id),
+      );
+      if (exists) return old;
+
       return {
         ...old,
         pages: old.pages.map((page, index) => {

apps/client/src/features/page/tree/hooks/use-tree-mutation.ts

@@ -19,7 +19,6 @@ import {
 } from "@/features/page/queries/page-query.ts";
 import { buildPageUrl } from "@/features/page/page.utils.ts";
 import { getSpaceUrl } from "@/lib/config.ts";
-import { useQueryEmit } from "@/features/websocket/use-query-emit.ts";
 
 export type UseTreeMutation = {
   handleMove: (sourceId: string, op: DropOp) => Promise<void>;
@@ -41,12 +40,11 @@ export function useTreeMutation(spaceId: string): UseTreeMutation {
   const movePageMutation = useMovePageMutation();
   const navigate = useNavigate();
   const { spaceSlug, pageSlug } = useParams();
-  const emit = useQueryEmit();
 
   const handleMove = useCallback(
     async (sourceId: string, op: DropOp) => {
       const before = store.get(treeDataAtom);
-      const { tree: after, result } = treeModel.move(before, sourceId, op);
+      const { tree: after } = treeModel.move(before, sourceId, op);
       if (after === before) return;
 
       const payload = dropOpToMovePayload(before, sourceId, op);
@@ -112,22 +110,12 @@ export function useTreeMutation(spaceId: string): UseTreeMutation {
         pageData,
       );
 
-      setTimeout(() => {
-        emit({
-          operation: "moveTreeNode",
-          spaceId: spaceId,
-          payload: {
-            id: sourceId,
-            parentId: payload.parentPageId,
-            oldParentId,
-            index: result.index,
-            position: payload.position,
-            pageData,
-          },
-        });
-      }, 50);
+      // Realtime broadcast is now server-authoritative: the server emits
+      // `moveTreeNode` to the space room on PAGE_MOVED. The old client relay
+      // (emit + setTimeout(50)) was removed; the optimistic local update above
+      // stays for instant feedback to the author.
     },
-    [setData, store, movePageMutation, spaceId, emit, t],
+    [setData, store, movePageMutation, spaceId, t],
   );
 
   const handleCreate = useCallback(
@@ -166,20 +154,23 @@ export function useTreeMutation(spaceId: string): UseTreeMutation {
         lastIndex = parent?.children?.length ?? 0;
       }
 
-      setData((prev) => treeModel.insert(prev, parentId, newNode, lastIndex));
-
-      setTimeout(() => {
-        emit({
-          operation: "addTreeNode",
-          spaceId,
-          payload: {
-            parentId,
-            index: lastIndex,
-            data: newNode,
-          },
-        });
-      }, 50);
+      // Idempotent by id: the tree is server-authoritative and the server's
+      // `addTreeNode` broadcast (now ~ms over same-origin) can win the race and
+      // insert this node before this optimistic update runs. Inserting again
+      // un-guarded would duplicate the row in the author's sidebar. Mirror the
+      // `addTreeNode` socket guard: skip when the node already exists. The
+      // optimistic node's id IS the real created page id (createdPage.id), so
+      // the ids match exactly regardless of which path runs first.
+      setData((prev) => {
+        if (treeModel.find(prev, newNode.id)) return prev;
+        return treeModel.insert(prev, parentId, newNode, lastIndex);
+      });
 
+      // Realtime broadcast is now server-authoritative: the server emits
+      // `addTreeNode` to the space room on PAGE_CREATED. The old client relay
+      // (emit + setTimeout(50)) was removed; the optimistic insert above stays
+      // for instant feedback to the author (the server event is idempotent and
+      // a no-op for the author whose node already exists).
       const pageUrl = buildPageUrl(
         spaceSlug,
         createdPage.slugId,
@@ -187,7 +178,7 @@ export function useTreeMutation(spaceId: string): UseTreeMutation {
       );
       navigate(pageUrl);
     },
-    [spaceId, createPageMutation, setData, store, emit, navigate, spaceSlug],
+    [spaceId, createPageMutation, setData, store, navigate, spaceSlug],
   );
 
   const handleRename = useCallback(
@@ -238,19 +229,15 @@ export function useTreeMutation(spaceId: string): UseTreeMutation {
           navigate(getSpaceUrl(spaceSlug));
         }
 
-        setTimeout(() => {
-          if (!node) return;
-          emit({
-            operation: "deleteTreeNode",
-            spaceId,
-            payload: { node },
-          });
-        }, 50);
+        // Realtime broadcast is now server-authoritative: the server emits
+        // `deleteTreeNode` to the space room on PAGE_SOFT_DELETED. The old
+        // client relay (emit + setTimeout(50)) was removed; the optimistic
+        // removal above stays for instant feedback to the author.
       } catch (error) {
         console.error("Failed to delete page:", error);
       }
     },
-    [removePageMutation, setData, store, pageSlug, navigate, spaceSlug, emit, spaceId],
+    [removePageMutation, setData, store, pageSlug, navigate, spaceSlug],
   );
 
   return { handleMove, handleCreate, handleRename, handleDelete };

apps/client/src/features/page/tree/model/tree-model.test.ts

@@ -128,6 +128,260 @@ describe('treeModel.insert', () => {
   });
 });
 
+describe('treeModel.insertByPosition', () => {
+  // Server-authoritative broadcasts ship the node's fractional `position`; the
+  // receiver inserts among already-loaded siblings ordered by `position`.
+  type P = TreeNode<{ name: string; position?: string }>;
+
+  const roots: P[] = [
+    { id: 'a', name: 'A', position: 'a0' },
+    { id: 'b', name: 'B', position: 'a2' },
+    { id: 'c', name: 'C', position: 'a4' },
+  ];
+
+  it('inserts a root node in position order (middle)', () => {
+    const node: P = { id: 'x', name: 'X', position: 'a3' };
+    const t = treeModel.insertByPosition(roots, null, node);
+    expect(t.map((n) => n.id)).toEqual(['a', 'b', 'x', 'c']);
+  });
+
+  it('inserts a root node at the front when its position sorts first', () => {
+    const node: P = { id: 'x', name: 'X', position: 'a-' };
+    const t = treeModel.insertByPosition(roots, null, node);
+    expect(t.map((n) => n.id)).toEqual(['x', 'a', 'b', 'c']);
+  });
+
+  it('appends a root node when its position sorts last', () => {
+    const node: P = { id: 'x', name: 'X', position: 'a9' };
+    const t = treeModel.insertByPosition(roots, null, node);
+    expect(t.map((n) => n.id)).toEqual(['a', 'b', 'c', 'x']);
+  });
+
+  it('produces the same order regardless of which siblings are loaded', () => {
+    // Client 1 loaded all siblings; client 2 only loaded a subset. The inserted
+    // node lands in a consistent relative position for both.
+    const full: P[] = roots;
+    const partial: P[] = [roots[0], roots[2]]; // a, c (b not loaded)
+    const node: P = { id: 'x', name: 'X', position: 'a3' };
+
+    expect(
+      treeModel.insertByPosition(full, null, node).map((n) => n.id),
+    ).toEqual(['a', 'b', 'x', 'c']);
+    expect(
+      treeModel.insertByPosition(partial, null, node).map((n) => n.id),
+    ).toEqual(['a', 'x', 'c']);
+  });
+
+  it('inserts a child in position order under the parent', () => {
+    const tree: P[] = [
+      {
+        id: 'p',
+        name: 'P',
+        position: 'a0',
+        children: [
+          { id: 'p1', name: 'P1', position: 'a0' },
+          { id: 'p2', name: 'P2', position: 'a2' },
+        ],
+      },
+    ];
+    const node: P = { id: 'p15', name: 'P1.5', position: 'a1' };
+    const t = treeModel.insertByPosition(tree, 'p', node);
+    expect(treeModel.find(t, 'p')?.children?.map((n) => n.id)).toEqual([
+      'p1', 'p15', 'p2',
+    ]);
+  });
+
+  it('appends when the new node has no position', () => {
+    const node: P = { id: 'x', name: 'X' };
+    const t = treeModel.insertByPosition(roots, null, node);
+    expect(t.map((n) => n.id)).toEqual(['a', 'b', 'c', 'x']);
+  });
+});
+
+// addTreeNode idempotency: the receiver early-returns when the node id already
+// exists, so re-delivery (or the author's optimistic node) is never duplicated.
+// This guards the find-then-skip contract insertByPosition relies on.
+describe('addTreeNode idempotency (find-then-skip)', () => {
+  type P = TreeNode<{ name: string; position?: string }>;
+
+  const applyAddTreeNode = (tree: P[], node: P): P[] => {
+    if (treeModel.find(tree, node.id)) return tree;
+    return treeModel.insertByPosition(tree, null, node);
+  };
+
+  it('does not insert a duplicate when the id already exists', () => {
+    const tree: P[] = [{ id: 'a', name: 'A', position: 'a0' }];
+    const node: P = { id: 'a', name: 'A again', position: 'a5' };
+    const t1 = applyAddTreeNode(tree, node);
+    expect(t1).toBe(tree);
+    expect(t1.map((n) => n.id)).toEqual(['a']);
+  });
+
+  it('inserts once, then is a no-op on repeat delivery', () => {
+    let tree: P[] = [{ id: 'a', name: 'A', position: 'a0' }];
+    const node: P = { id: 'x', name: 'X', position: 'a5' };
+    tree = applyAddTreeNode(tree, node);
+    expect(tree.map((n) => n.id)).toEqual(['a', 'x']);
+    const again = applyAddTreeNode(tree, node);
+    expect(again).toBe(tree);
+    expect(again.filter((n) => n.id === 'x')).toHaveLength(1);
+  });
+});
+
+// handleCreate optimistic-insert idempotency: the author's optimistic insert is
+// now guarded by `treeModel.find` (same contract as the addTreeNode socket
+// handler) because the server's broadcast can win the race and insert the node
+// first. Whichever runs first inserts; the second is a no-op. Exactly one row.
+describe('handleCreate optimistic-insert idempotency (find-then-skip)', () => {
+  // Mirrors the guarded optimistic insert in use-tree-mutation handleCreate.
+  const applyOptimisticInsert = (
+    tree: N[],
+    parentId: string | null,
+    node: N,
+    index: number,
+  ): N[] => {
+    if (treeModel.find(tree, node.id)) return tree;
+    return treeModel.insert(tree, parentId, node, index);
+  };
+
+  // Mirrors the addTreeNode socket handler guard.
+  const applyAddTreeNode = (tree: N[], parentId: string | null, node: N): N[] => {
+    if (treeModel.find(tree, node.id)) return tree;
+    return treeModel.insert(tree, parentId, node);
+  };
+
+  const created: N = { id: 'new', name: '' };
+
+  it('optimistic insert is a no-op when server addTreeNode already inserted it', () => {
+    // Reverse-of-reverse race: server wins.
+    const afterServer = applyAddTreeNode(fixture, null, created);
+    expect(afterServer.filter((n) => n.id === 'new')).toHaveLength(1);
+    const afterOptimistic = applyOptimisticInsert(
+      afterServer,
+      null,
+      created,
+      afterServer.length,
+    );
+    expect(afterOptimistic).toBe(afterServer); // skipped
+    expect(afterOptimistic.filter((n) => n.id === 'new')).toHaveLength(1);
+  });
+
+  it('server addTreeNode is a no-op when optimistic insert already ran (optimistic-first)', () => {
+    const afterOptimistic = applyOptimisticInsert(fixture, null, created, fixture.length);
+    expect(afterOptimistic.filter((n) => n.id === 'new')).toHaveLength(1);
+    const afterServer = applyAddTreeNode(afterOptimistic, null, created);
+    expect(afterServer).toBe(afterOptimistic); // skipped
+    expect(afterServer.filter((n) => n.id === 'new')).toHaveLength(1);
+  });
+
+  it('inserts exactly once when only the optimistic path runs', () => {
+    const t = applyOptimisticInsert(fixture, 'a', { id: 'a3', name: '' }, 2);
+    expect(treeModel.find(t, 'a')?.children?.filter((n) => n.id === 'a3')).toHaveLength(1);
+  });
+});
+
+// moveTreeNode socket-handler semantics: the receiver must place the moved node
+// by `position` (NOT index 0) and apply the `pageData` the payload carries so a
+// moved node's title/icon/chevron stay correct. This mirrors the reducer in
+// use-tree-socket.ts so the contract is unit-tested without rendering the hook.
+describe('moveTreeNode handler (place by position + apply pageData)', () => {
+  type P = TreeNode<{
+    name: string;
+    position?: string;
+    icon?: string;
+    hasChildren?: boolean;
+    parentPageId?: string | null;
+  }>;
+
+  const applyMoveTreeNode = (
+    tree: P[],
+    payload: {
+      id: string;
+      parentId: string | null;
+      position: string;
+      pageData?: { title?: string | null; icon?: string | null; hasChildren?: boolean };
+    },
+  ): P[] => {
+    if (!treeModel.find(tree, payload.id)) return tree;
+    const placed = treeModel.placeByPosition(tree, payload.id, {
+      parentId: payload.parentId,
+      position: payload.position,
+    });
+    if (placed === tree) return treeModel.remove(tree, payload.id);
+    const patch: Partial<P> = {
+      position: payload.position,
+      parentPageId: payload.parentId,
+    } as Partial<P>;
+    const pd = payload.pageData;
+    if (pd) {
+      if (pd.title !== undefined) (patch as { name?: string }).name = pd.title ?? '';
+      if (pd.icon !== undefined) (patch as { icon?: string }).icon = pd.icon ?? undefined;
+      if (pd.hasChildren !== undefined)
+        (patch as { hasChildren?: boolean }).hasChildren = pd.hasChildren;
+    }
+    return treeModel.update(placed, payload.id, patch);
+  };
+
+  const tree: P[] = [
+    {
+      id: 'dst',
+      name: 'DST',
+      position: 'a0',
+      children: [
+        { id: 'c1', name: 'C1', position: 'a1' },
+        { id: 'c2', name: 'C2', position: 'a3' },
+        { id: 'c3', name: 'C3', position: 'a5' },
+      ],
+    },
+    { id: 'src', name: 'SRC', position: 'a9' },
+  ];
+
+  it('lands the moved node in the correct MIDDLE slot, not at index 0', () => {
+    const t = applyMoveTreeNode(tree, {
+      id: 'src',
+      parentId: 'dst',
+      position: 'a4',
+    });
+    expect(treeModel.find(t, 'dst')?.children?.map((n) => n.id)).toEqual([
+      'c1', 'c2', 'src', 'c3',
+    ]);
+  });
+
+  it('lands the moved node at the END when position sorts last', () => {
+    const t = applyMoveTreeNode(tree, {
+      id: 'src',
+      parentId: 'dst',
+      position: 'a8',
+    });
+    expect(treeModel.find(t, 'dst')?.children?.map((n) => n.id)).toEqual([
+      'c1', 'c2', 'c3', 'src',
+    ]);
+  });
+
+  it('applies pageData (title/icon/hasChildren) to the moved node', () => {
+    const t = applyMoveTreeNode(tree, {
+      id: 'src',
+      parentId: 'dst',
+      position: 'a4',
+      pageData: { title: 'Renamed', icon: '🔥', hasChildren: true },
+    });
+    const moved = treeModel.find(t, 'src');
+    expect(moved?.name).toBe('Renamed');
+    expect(moved?.icon).toBe('🔥');
+    expect(moved?.hasChildren).toBe(true);
+    expect(moved?.position).toBe('a4');
+  });
+
+  it('falls back to removing the node when the destination parent is not loaded', () => {
+    const t = applyMoveTreeNode(tree, {
+      id: 'src',
+      parentId: 'not-loaded',
+      position: 'a4',
+    });
+    expect(treeModel.find(t, 'src')).toBeNull();
+  });
+});
+
 describe('treeModel.remove', () => {
   it('removes a leaf', () => {
     const t = treeModel.remove(fixture, 'a2');
@@ -240,6 +494,118 @@ describe('treeModel.place', () => {
   });
 });
 
+describe('treeModel.placeByPosition', () => {
+  // Server-authoritative `moveTreeNode` ships the moved node's fractional
+  // `position`; the receiver must sort it into the correct slot among the new
+  // siblings — NOT drop it at index 0.
+  type P = TreeNode<{ name: string; position?: string }>;
+
+  const tree: P[] = [
+    {
+      id: 'dst',
+      name: 'DST',
+      position: 'a0',
+      children: [
+        { id: 'c1', name: 'C1', position: 'a1' },
+        { id: 'c2', name: 'C2', position: 'a3' },
+        { id: 'c3', name: 'C3', position: 'a5' },
+      ],
+    },
+    { id: 'src', name: 'SRC', position: 'a9' },
+  ];
+
+  it('places the moved node in the MIDDLE of new siblings by position', () => {
+    const t = treeModel.placeByPosition(tree, 'src', {
+      parentId: 'dst',
+      position: 'a4',
+    });
+    expect(treeModel.find(t, 'dst')?.children?.map((n) => n.id)).toEqual([
+      'c1', 'c2', 'src', 'c3',
+    ]);
+  });
+
+  it('places the moved node at the END when its position sorts last', () => {
+    const t = treeModel.placeByPosition(tree, 'src', {
+      parentId: 'dst',
+      position: 'a8',
+    });
+    expect(treeModel.find(t, 'dst')?.children?.map((n) => n.id)).toEqual([
+      'c1', 'c2', 'c3', 'src',
+    ]);
+  });
+
+  it('places the moved node at the FRONT only when its position sorts first', () => {
+    const t = treeModel.placeByPosition(tree, 'src', {
+      parentId: 'dst',
+      position: 'a0',
+    });
+    expect(treeModel.find(t, 'dst')?.children?.map((n) => n.id)).toEqual([
+      'src', 'c1', 'c2', 'c3',
+    ]);
+  });
+
+  it('stamps the authoritative position onto the moved node', () => {
+    const t = treeModel.placeByPosition(tree, 'src', {
+      parentId: 'dst',
+      position: 'a4',
+    });
+    expect(treeModel.find(t, 'src')?.position).toBe('a4');
+  });
+
+  it('reorders within the same parent by position (not to index 0)', () => {
+    const same: P[] = [
+      {
+        id: 'p',
+        name: 'P',
+        position: 'a0',
+        children: [
+          { id: 'x', name: 'X', position: 'a1' },
+          { id: 'y', name: 'Y', position: 'a2' },
+          { id: 'z', name: 'Z', position: 'a3' },
+        ],
+      },
+    ];
+    // Move x to between y and z.
+    const t = treeModel.placeByPosition(same, 'x', {
+      parentId: 'p',
+      position: 'a25',
+    });
+    expect(treeModel.find(t, 'p')?.children?.map((n) => n.id)).toEqual([
+      'y', 'x', 'z',
+    ]);
+  });
+
+  it('returns same array reference for unknown source', () => {
+    expect(
+      treeModel.placeByPosition(tree, 'ghost', { parentId: 'dst', position: 'a4' }),
+    ).toBe(tree);
+  });
+
+  it('returns same array reference when destination parent is not loaded', () => {
+    expect(
+      treeModel.placeByPosition(tree, 'src', { parentId: 'ghost', position: 'a4' }),
+    ).toBe(tree);
+  });
+
+  it('moves a node to root by position', () => {
+    const roots: P[] = [
+      { id: 'r1', name: 'R1', position: 'a1' },
+      { id: 'r2', name: 'R2', position: 'a5' },
+      {
+        id: 'rp',
+        name: 'RP',
+        position: 'a7',
+        children: [{ id: 'child', name: 'CHILD', position: 'a1' }],
+      },
+    ];
+    const t = treeModel.placeByPosition(roots, 'child', {
+      parentId: null,
+      position: 'a3',
+    });
+    expect(t.map((n) => n.id)).toEqual(['r1', 'child', 'r2', 'rp']);
+  });
+});
+
 describe('treeModel.move', () => {
   it('reorder-before within same parent: moves source to target index', () => {
     const { tree: t, result } = treeModel.move(fixture, 'a2', {

apps/client/src/features/page/tree/model/tree-model.ts

@@ -98,6 +98,35 @@ export const treeModel = {
     return touched ? out : tree;
   },
 
+  // Position-aware insert for server-authoritative broadcasts. The server does
+  // not know each receiver's local index (clients have different loaded sets and
+  // the root list is paginated), so it sends the node's fractional `position`.
+  // We insert among the already-loaded siblings ordered by `position` so the
+  // order is consistent across clients regardless of which nodes they loaded.
+  // Falls back to appending when `position` is missing.
+  insertByPosition<T extends { position?: string }>(
+    tree: TreeNode<T>[],
+    parentId: string | null,
+    node: TreeNode<T>,
+  ): TreeNode<T>[] {
+    const index = (siblings: TreeNode<T>[]): number => {
+      const pos = node.position;
+      if (pos == null) return siblings.length;
+      // First sibling whose position sorts after the new node's position.
+      const at = siblings.findIndex(
+        (s) => s.position != null && s.position > pos,
+      );
+      return at === -1 ? siblings.length : at;
+    };
+
+    if (parentId === null) {
+      return treeModel.insert(tree, null, node, index(tree));
+    }
+    const parent = treeModel.find(tree, parentId);
+    const kids = (parent?.children as TreeNode<T>[] | undefined) ?? [];
+    return treeModel.insert(tree, parentId, node, index(kids));
+  },
+
   remove<T extends object>(tree: TreeNode<T>[], id: string): TreeNode<T>[] {
     let touched = false;
     const walk = (nodes: TreeNode<T>[]): TreeNode<T>[] => {
@@ -186,6 +215,30 @@ export const treeModel = {
     return treeModel.insert(removed, to.parentId, source, to.index);
   },
 
+  // Position-aware move for server-authoritative `moveTreeNode` broadcasts. Like
+  // `place`, but instead of an absolute index (which the sender computed against
+  // its own loaded set), it inserts the moved node among the destination's
+  // already-loaded siblings ordered by the node's fractional `position`. This
+  // keeps the visible order correct for every receiver — `place(..., index: 0)`
+  // would wrongly drop the node at the TOP of its new sibling list.
+  // Returns the same array reference (like `place`) when the source is missing
+  // or the destination parent isn't loaded on this client, so callers can detect
+  // that and fall back to removing the node.
+  placeByPosition<T extends { position?: string }>(
+    tree: TreeNode<T>[],
+    sourceId: string,
+    to: { parentId: string | null; position?: string },
+  ): TreeNode<T>[] {
+    const source = treeModel.find(tree, sourceId);
+    if (!source) return tree;
+    if (to.parentId !== null && !treeModel.find(tree, to.parentId)) return tree;
+    const removed = treeModel.remove(tree, sourceId);
+    // Reuse the same position-ordered insertion as `insertByPosition` by
+    // stamping the authoritative position onto the moved node first.
+    const positioned = { ...source, position: to.position } as TreeNode<T>;
+    return treeModel.insertByPosition(removed, to.parentId, positioned);
+  },
+
   move<T extends object>(
     tree: TreeNode<T>[],
     sourceId: string,

apps/client/src/features/page/tree/utils/utils.test.ts

@@ -0,0 +1,40 @@
+import { describe, it, expect } from "vitest";
+import { buildTree } from "./utils";
+import type { IPage } from "@/features/page/types/page.types.ts";
+
+function page(id: string, position: string): IPage {
+  return {
+    id,
+    slugId: `slug-${id}`,
+    title: id.toUpperCase(),
+    icon: "",
+    position,
+    hasChildren: false,
+    spaceId: "space-1",
+    parentPageId: null as unknown as string,
+  } as IPage;
+}
+
+describe("buildTree", () => {
+  it("builds one node per unique page", () => {
+    const tree = buildTree([page("a", "a1"), page("b", "a2")]);
+    expect(tree.map((n) => n.id)).toEqual(["a", "b"]);
+  });
+
+  it("dedups a duplicate id so the tree has no duplicate node", () => {
+    // A realtime cache write could append a page twice; buildTree must not emit
+    // two references to the same node (which would crash the sidebar render with
+    // a duplicate React key).
+    const tree = buildTree([
+      page("a", "a1"),
+      page("b", "a2"),
+      page("a", "a1"), // duplicate id
+    ]);
+
+    expect(tree).toHaveLength(2);
+    expect(tree.map((n) => n.id).sort()).toEqual(["a", "b"]);
+    // No id appears more than once.
+    const ids = tree.map((n) => n.id);
+    expect(new Set(ids).size).toBe(ids.length);
+  });
+});

apps/client/src/features/page/tree/utils/utils.ts

@@ -29,7 +29,14 @@ export function buildTree(pages: IPage[]): SpaceTreeNode[] {
     };
   });
 
+  // Defense-in-depth: a duplicate id in `pages` would push two references to the
+  // same node, producing a duplicate React key that crashes the sidebar render.
+  // Track ids we've already pushed and skip repeats so a stray duplicate from a
+  // realtime cache write can never break the tree.
+  const seen = new Set<string>();
   pages.forEach((page) => {
+    if (seen.has(page.id)) return;
+    seen.add(page.id);
     tree.push(pageMap[page.id]);
   });
 

apps/client/src/features/websocket/use-tree-socket.ts

@@ -54,13 +54,17 @@ export const useTreeSocket = () => {
           break;
         case "addTreeNode":
           setTreeData((prev) => {
+            // Idempotent: the author already inserted the node optimistically,
+            // and a node may be re-delivered — never insert a duplicate id.
             if (treeModel.find(prev, event.payload.data.id)) return prev;
             const newParentId = event.payload.parentId as string | null;
-            let next = treeModel.insert(
+            // Insert by `position` among already-loaded siblings (not the
+            // sender's absolute index) so order is consistent across clients
+            // with different loaded sets.
+            let next = treeModel.insertByPosition(
               prev,
               newParentId,
               event.payload.data,
-              event.payload.index,
             );
             // Mirror the emitter: flip new parent's hasChildren to true so
             // the chevron renders on the receiver.
@@ -80,22 +84,50 @@ export const useTreeSocket = () => {
               (sourceBefore as SpaceTreeNode).parentPageId ?? null;
             const newParentId = event.payload.parentId as string | null;
 
-            const placed = treeModel.place(prev, event.payload.id, {
+            // Place the node by its fractional `position` among the new
+            // siblings — NOT by the sender's absolute `index` (the sender
+            // computed that against its own loaded set, which differs from
+            // this receiver's). Using the position keeps the visible order
+            // correct on every client; placing at `index: 0` would wrongly
+            // drop reordered/moved nodes at the top of their new sibling list.
+            const placed = treeModel.placeByPosition(prev, event.payload.id, {
               parentId: newParentId,
-              index: event.payload.index,
+              position: event.payload.position,
             });
-            // `place` silently returns the same reference if the destination
-            // parent isn't loaded on this client. Falling back to removing the
-            // source keeps the UI consistent (the source will reappear when
-            // the user expands the new parent and lazy-load fetches it).
+            // `placeByPosition` silently returns the same reference if the
+            // destination parent isn't loaded on this client. Falling back to
+            // removing the source keeps the UI consistent (the source will
+            // reappear when the user expands the new parent and lazy-load
+            // fetches it).
             if (placed === prev) {
               return treeModel.remove(prev, event.payload.id);
             }
 
-            let next = treeModel.update(placed, event.payload.id, {
+            // Apply the authoritative node fields the move payload carries
+            // (`pageData`) so receivers don't keep a stale title/icon/chevron
+            // on the moved node. `placeByPosition` already set `position`.
+            const pageData = event.payload.pageData as
+              | {
+                  title?: string | null;
+                  icon?: string | null;
+                  hasChildren?: boolean;
+                }
+              | undefined;
+            const patch: Partial<SpaceTreeNode> = {
               position: event.payload.position,
-              parentPageId: newParentId,
-            } as Partial<SpaceTreeNode>);
+              // Honest type: a root move has a null parent, so this is
+              // `string | null`, not always `string`.
+              parentPageId: newParentId as string | null,
+            };
+            if (pageData) {
+              // The tree node stores the title as `name`.
+              if (pageData.title !== undefined) patch.name = pageData.title ?? "";
+              if (pageData.icon !== undefined)
+                patch.icon = pageData.icon ?? undefined;
+              if (pageData.hasChildren !== undefined)
+                patch.hasChildren = pageData.hasChildren;
+            }
+            let next = treeModel.update(placed, event.payload.id, patch);
 
             // Mirror the emitter's hasChildren bookkeeping so both clients
             // converge to the same chevron state.