fix(tree): stop silent page loss on move-to-unloaded-parent + reconnect ghost roots (#159)

Two confirmed P1 data-loss findings in the sidebar tree sync.

#1 — Move into an unloaded/collapsed parent silently dropped pages. When a
moveTreeNode (or addTreeNode) broadcast targeted a parent whose children were
NOT yet lazy-loaded, `insertByPosition` did `kids = parent.children ?? []` and
inserted the moved node, MATERIALIZING a misleading partial child list
(`[movedNode]`) out of an unloaded (`children === undefined`) parent. The
lazy-load gate fetches only when children are absent/empty, so it then refused
to fetch — leaving the parent showing ONLY the moved node and HIDING all its
other real children (and, when the parent wasn't in the tree at all, the node
was removed and never re-fetched). Fix: `insertByPosition` distinguishes
`children === undefined` (not loaded) from `[]` (loaded-empty) and, for an
unloaded parent, does NOT insert — it leaves children unloaded and just flags
`hasChildren`, so expanding fetches the FULL set (including the moved/added
node) via the existing lazy-load.

#2 — After a socket reconnect, a deleted/moved-away root lingered as a 404
"ghost". `mergeRootTrees` was append-only: it kept every previously-loaded root
and only added new ones, so a root removed during the missed-events gap was
never dropped. It runs only once all root pages are fetched, so the incoming
list is the authoritative complete root set — fix reconciles to it (drop roots
absent from incoming) while PRESERVING each surviving root's lazy-loaded
subtree and refreshing its own fields.

Tests: insertByPosition unloaded-vs-loaded-empty parent; the move reducer
keeps a collapsed destination lazy-loadable instead of partial; mergeRootTrees
drops a ghost root, preserves a surviving subtree, adds new roots, refreshes
fields. The existing "remove when parent not in tree" reducer test still holds.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

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

@@ -1,4 +1,4 @@
-import type { TreeNode, SiblingsInfo } from './tree-model.types';
+import type { TreeNode, SiblingsInfo } from "./tree-model.types";
 
 function findInternal<T extends object>(
   nodes: TreeNode<T>[],
@@ -19,7 +19,10 @@ export const treeModel = {
     return findInternal(tree, id)?.node ?? null;
   },
 
-  path<T extends object>(tree: TreeNode<T>[], id: string): TreeNode<T>[] | null {
+  path<T extends object>(
+    tree: TreeNode<T>[],
+    id: string,
+  ): TreeNode<T>[] | null {
     const found = findInternal(tree, id);
     if (!found) return null;
     return [...found.parents, found.node];
@@ -123,6 +126,23 @@ export const treeModel = {
       return treeModel.insert(tree, null, node, index(tree));
     }
     const parent = treeModel.find(tree, parentId);
+    // The parent is in the tree but its children have NOT been lazy-loaded yet
+    // (`children === undefined`, distinct from a loaded-but-empty `[]`). Inserting
+    // here would MATERIALIZE a misleading partial child list (`[node]`) that
+    // defeats the lazy-load gate — which fetches only when children are
+    // absent/empty — so the parent's OTHER real children would never load and the
+    // moved/added node would be the only one shown (a silent data loss, #159 #1).
+    // Instead, leave the children unloaded and just flag `hasChildren` so the
+    // chevron appears; expanding fetches the FULL set (including this node).
+    if (parent && parent.children === undefined) {
+      return treeModel.update(
+        tree,
+        parentId,
+        // hasChildren is not part of the generic T constraint; tree nodes carry
+        // it. Cast narrowly so this stays a single, well-understood exception.
+        { hasChildren: true } as unknown as Omit<Partial<T>, "id" | "children">,
+      );
+    }
     const kids = (parent?.children as TreeNode<T>[] | undefined) ?? [];
     return treeModel.insert(tree, parentId, node, index(kids));
   },
@@ -242,9 +262,10 @@ export const treeModel = {
   move<T extends object>(
     tree: TreeNode<T>[],
     sourceId: string,
-    op: import('./tree-model.types').DropOp,
+    op: import("./tree-model.types").DropOp,
-  ): { tree: TreeNode<T>[]; result: import('./tree-model.types').DropResult } {
+  ): { tree: TreeNode<T>[]; result: import("./tree-model.types").DropResult } {
-    if (sourceId === op.targetId) return { tree, result: { parentId: null, index: 0 } };
+    if (sourceId === op.targetId)
+      return { tree, result: { parentId: null, index: 0 } };
     if (!treeModel.find(tree, sourceId) || !treeModel.find(tree, op.targetId)) {
       return { tree, result: { parentId: null, index: 0 } };
     }
@@ -255,7 +276,7 @@ export const treeModel = {
     let parentId: string | null;
     let index: number;
 
-    if (op.kind === 'make-child') {
+    if (op.kind === "make-child") {
       parentId = op.targetId;
       const target = treeModel.find(tree, op.targetId)!;
       index = target.children?.length ?? 0;
@@ -264,9 +285,8 @@ export const treeModel = {
       parentId = info.parentId;
       const sourceInfo = treeModel.siblingsOf(tree, sourceId)!;
       const sameParent = sourceInfo.parentId === parentId;
-      const adjust =
+      const adjust = sameParent && sourceInfo.index < info.index ? -1 : 0;
-        sameParent && sourceInfo.index < info.index ? -1 : 0;
+      index = info.index + adjust + (op.kind === "reorder-after" ? 1 : 0);
-      index = info.index + adjust + (op.kind === 'reorder-after' ? 1 : 0);
     }
 
     const next = treeModel.place(tree, sourceId, { parentId, index });

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

@@ -6,6 +6,7 @@ import {
   collectBranchIds,
   openBranches,
   closeIds,
+  mergeRootTrees,
 } from "./utils";
 import type { IPage } from "@/features/page/types/page.types.ts";
 import type { SpaceTreeNode } from "@/features/page/tree/types.ts";
@@ -44,10 +45,7 @@ function flatNode(
 }
 
 // Nested SpaceTreeNode factory for collectAllIds / collectBranchIds.
-function treeNode(
+function treeNode(id: string, children: SpaceTreeNode[] = []): SpaceTreeNode {
-  id: string,
-  children: SpaceTreeNode[] = [],
-): SpaceTreeNode {
   return {
     id,
     slugId: `slug-${id}`,
@@ -94,11 +92,7 @@ describe("collectBranchIds", () => {
       ]),
       treeNode("root2", [treeNode("leaf3")]),
     ];
-    expect(collectBranchIds(tree).sort()).toEqual([
+    expect(collectBranchIds(tree).sort()).toEqual(["branch1", "root", "root2"]);
-      "branch1",
-      "root",
-      "root2",
-    ]);
   });
 
   it("returns [] for a leaf-only tree", () => {
@@ -273,3 +267,57 @@ describe("closeIds", () => {
     expect(twice).toEqual({ keep: true, a: false, b: false });
   });
 });
+
+describe("mergeRootTrees (#159 #2 reconnect reconcile)", () => {
+  // Root node with a position and optional already-loaded children.
+  function root(
+    id: string,
+    position: string,
+    children?: SpaceTreeNode[],
+  ): SpaceTreeNode {
+    return {
+      id,
+      slugId: `slug-${id}`,
+      name: id.toUpperCase(),
+      icon: undefined,
+      position,
+      spaceId: "space-1",
+      parentPageId: null as unknown as string,
+      hasChildren: !!children?.length,
+      children: children as SpaceTreeNode[],
+    };
+  }
+
+  it("DROPS a stale root that is absent from the incoming (authoritative) set", () => {
+    // 'ghost' was a root before the gap; the server's current roots no longer
+    // include it (deleted / moved under another page). It must not linger.
+    const prev = [root("a", "a0"), root("ghost", "a2"), root("b", "a4")];
+    const incoming = [root("a", "a0"), root("b", "a4")];
+    const merged = mergeRootTrees(prev, incoming);
+    expect(merged.map((n) => n.id)).toEqual(["a", "b"]);
+    expect(merged.find((n) => n.id === "ghost")).toBeUndefined();
+  });
+
+  it("PRESERVES a surviving root's lazy-loaded children (subtree not lost on refetch)", () => {
+    const loadedChild = root("a1", "a0");
+    const prev = [root("a", "a0", [loadedChild])];
+    // The root query returns only top-level roots (no children).
+    const incoming = [root("a", "a0")];
+    const merged = mergeRootTrees(prev, incoming);
+    expect(merged[0].children?.map((c) => c.id)).toEqual(["a1"]);
+  });
+
+  it("ADDS a new incoming root", () => {
+    const prev = [root("a", "a0")];
+    const incoming = [root("a", "a0"), root("new", "a2")];
+    const merged = mergeRootTrees(prev, incoming);
+    expect(merged.map((n) => n.id)).toEqual(["a", "new"]);
+  });
+
+  it("REFRESHES a surviving root's own fields from the incoming copy (e.g. rename)", () => {
+    const prev = [{ ...root("a", "a0"), name: "OLD" }];
+    const incoming = [{ ...root("a", "a0"), name: "NEW" }];
+    const merged = mergeRootTrees(prev, incoming);
+    expect(merged[0].name).toBe("NEW");
+  });
+});

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

@@ -214,21 +214,36 @@ export function appendNodeChildren(
 }
 
 /**
- * Merge root nodes; keep existing ones intact, append new ones,
+ * Reconcile the loaded root nodes to the authoritative INCOMING set (the
+ * server's complete current roots for the space), preserving any lazy-loaded
+ * children/subtree of a root that still exists.
+ *
+ * This runs only once all root pages are fetched, so `incomingRoots` is the full
+ * server root set and is authoritative for WHICH roots exist:
+ *  - a root in BOTH: kept, with its own fields refreshed from `incoming` (so a
+ *    rename/move during a gap shows) while PRESERVING its previously lazy-loaded
+ *    `children` (expanded subtrees + open-state survive a refetch);
+ *  - a root only in `incoming`: a new root, added as-is;
+ *  - a root only in `prev`: it was DELETED or moved under another page while we
+ *    were not receiving events (e.g. a socket reconnect after a sleep/wifi gap).
+ *    It is DROPPED instead of lingering as a 404 "ghost" root (#159 #2). The old
+ *    append-only merge kept it forever.
  */
 export function mergeRootTrees(
   prevRoots: SpaceTreeNode[],
   incomingRoots: SpaceTreeNode[],
 ): SpaceTreeNode[] {
-  const seen = new Set(prevRoots.map((r) => r.id));
+  const prevById = new Map(prevRoots.map((r) => [r.id, r]));
 
-  // add new roots that were not present before
+  const reconciled = incomingRoots.map((incoming) => {
-  const merged = [...prevRoots];
+    const prev = prevById.get(incoming.id);
-  incomingRoots.forEach((node) => {
+    // Preserve the previously loaded children/subtree (the root query returns
-    if (!seen.has(node.id)) merged.push(node);
+    // only top-level roots, so `incoming` carries no children); refresh the
+    // node's own fields from the authoritative incoming copy.
+    return prev ? { ...incoming, children: prev.children } : incoming;
   });
 
-  return sortPositionKeys(merged);
+  return sortPositionKeys(reconciled);
 }
 
 // Collect every node id in the tree (roots, branches, leaves). Used by

apps/client/src/features/websocket/tree-socket-reducers.test.ts

@@ -81,6 +81,38 @@ describe("applyMoveTreeNode", () => {
     ]);
   });
 
+  it("does NOT create a partial child list when the destination is loaded-but-collapsed (children unloaded) — keeps it lazy-loadable (#159)", () => {
+    // `dstCollapsed` is in the tree but its children were never lazy-loaded
+    // (children === undefined). The OLD behavior inserted `src` as the ONLY
+    // child ([src]), which defeated the lazy-load gate and HID the parent's
+    // other real children. Now the move leaves children unloaded (so expanding
+    // fetches the FULL set, including src) and just flags hasChildren.
+    const tree: SpaceTreeNode[] = [
+      node("dstCollapsed", {
+        position: "a0",
+        hasChildren: false,
+        children: undefined as unknown as SpaceTreeNode[],
+      }),
+      node("src", { position: "a9" }),
+    ];
+    const next = applyMoveTreeNode(tree, {
+      id: "src",
+      parentId: "dstCollapsed",
+      oldParentId: null,
+      index: 0,
+      position: "a4",
+      pageData: {},
+    });
+    const dst = treeModel.find(next, "dstCollapsed");
+    // Children stay unloaded -> the lazy-load gate fetches the FULL set (incl.
+    // src) on expand, rather than showing a misleading partial [src] list.
+    expect(dst?.children).toBeUndefined();
+    expect(dst?.hasChildren).toBe(true);
+    // src moved away from its old root slot (it lives under dstCollapsed
+    // server-side and reappears when the parent is expanded/loaded).
+    expect(next.map((n) => n.id)).not.toContain("src");
+  });
+
   it("flips the OLD parent's hasChildren to false when it is left childless", () => {
     // src is the only child of `old`; moving it to `dst` empties `old`.
     const tree: SpaceTreeNode[] = [
@@ -164,7 +196,9 @@ describe("applyDeleteTreeNode", () => {
             position: "a1",
             parentPageId: "p",
             hasChildren: true,
-            children: [node("grandchild", { position: "a1", parentPageId: "child" })],
+            children: [
+              node("grandchild", { position: "a1", parentPageId: "child" }),
+            ],
           }),
         ],
       }),