test(temporary-notes): cover the create race-guard, broadcast deadline + cache patch; unify page->tree-node mappers

Address review comment 2159 on the temporary-notes UI work.

Tests:
- tree-model: cover handleCreate's race-guard temporaryExpiresAt patch — (a)
  server node inserted WITHOUT a deadline + create response carries one => node
  gains the deadline; (b) node already has a deadline => not overwritten, prev
  returned by reference.
- ws-tree.service.spec: broadcastPageCreated now asserts the deadline is carried
  when present and pinned to null (`?? null`) when absent.
- page-embed-query (new spec): syncTemporaryExpiresInCache patches the in-tree
  node's temporaryExpiresAt, and leaves the atom value at the same reference when
  the id is absent from the loaded tree (no write).

Refactor (closes the drift bug-class at the root):
- Client: extract one canonical pageToTreeNode(page, overrides) mapper in
  tree/utils and route buildTree, handleCreate's optimistic insert, the restore
  mutation and the duplicate handler through it. Restore stays permanent (server
  nulls temporaryExpiresAt) and duplicate stays permanent (server arms no timer)
  — both now reflect the server without a reload, where before they dropped the
  field entirely.
- Server: extract one toTreeNodeSnapshot(page) helper called by both the
  PAGE_CREATED event enrichment (page.repo) and the addTreeNode broadcast
  (ws-tree.service), so the optional temporaryExpiresAt can't drift between the
  two literals.

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

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

@@ -0,0 +1,69 @@
+import { describe, it, expect, vi, beforeEach } from "vitest";
+import { getDefaultStore } from "jotai";
+
+// Mock the app entry so importing the query module doesn't boot the whole app
+// (it only needs queryClient's cache methods, which we stub here). The spies are
+// declared via vi.hoisted so they exist before the hoisted vi.mock factory runs.
+const { setQueryData, getQueryData, invalidateQueries } = vi.hoisted(() => ({
+  setQueryData: vi.fn(),
+  getQueryData: vi.fn(() => undefined as unknown),
+  invalidateQueries: vi.fn(),
+}));
+vi.mock("@/main.tsx", () => ({
+  queryClient: { setQueryData, getQueryData, invalidateQueries },
+}));
+
+import { syncTemporaryExpiresInCache } from "./page-embed-query";
+import { treeDataAtom } from "@/features/page/tree/atoms/tree-data-atom.ts";
+import { SpaceTreeNode } from "@/features/page/tree/types.ts";
+
+const mkNode = (id: string, slugId: string): SpaceTreeNode =>
+  ({
+    id,
+    slugId,
+    name: id,
+    position: "a0",
+    spaceId: "space-1",
+    parentPageId: null,
+    hasChildren: false,
+    children: [],
+  }) as unknown as SpaceTreeNode;
+
+describe("syncTemporaryExpiresInCache — treeDataAtom patch", () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+    getQueryData.mockReturnValue(undefined);
+  });
+
+  it("patches the in-tree node's temporaryExpiresAt (sidebar marker updates without reload)", () => {
+    const store = getDefaultStore();
+    const tree = [mkNode("p1", "slug-1"), mkNode("p2", "slug-2")];
+    store.set(treeDataAtom, tree);
+
+    const deadline = "2026-07-01T00:00:00.000Z";
+    syncTemporaryExpiresInCache({ id: "p1", slugId: "slug-1" }, deadline);
+
+    const next = store.get(treeDataAtom);
+    // A new atom value was written...
+    expect(next).not.toBe(tree);
+    // ...the matching node gained the deadline...
+    expect(next.find((n) => n.id === "p1")?.temporaryExpiresAt).toBe(deadline);
+    // ...and the untouched sibling is unchanged.
+    expect(next.find((n) => n.id === "p2")?.temporaryExpiresAt).toBeUndefined();
+  });
+
+  it("leaves the atom value at the SAME reference when the id is absent from the tree (no write)", () => {
+    const store = getDefaultStore();
+    const tree = [mkNode("p1", "slug-1")];
+    store.set(treeDataAtom, tree);
+
+    syncTemporaryExpiresInCache(
+      { id: "not-in-tree", slugId: "missing" },
+      "2026-07-01T00:00:00.000Z",
+    );
+
+    // treeModel.update is a no-op (same reference) for an unknown id, so the
+    // guard skips the store write entirely — same reference back.
+    expect(store.get(treeDataAtom)).toBe(tree);
+  });
+});

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

@@ -32,7 +32,7 @@ import {
 import { notifications } from "@mantine/notifications";
 import { IPagination, QueryParams } from "@/lib/types.ts";
 import { queryClient } from "@/main.tsx";
-import { buildTree } from "@/features/page/tree/utils";
+import { buildTree, pageToTreeNode } from "@/features/page/tree/utils";
 import { useEffect } from "react";
 import { validate as isValidUuid } from "uuid";
 import { useTranslation } from "react-i18next";
@@ -210,18 +210,15 @@ export function useRestorePageMutation() {
 
       // Check if the page already exists in the tree (it shouldn't)
       if (!treeModel.find(currentTree, restoredPage.id)) {
-        // Create the tree node data with hasChildren from backend
+        // Create the tree node data with hasChildren from backend. Routed
-        const nodeData: SpaceTreeNode = {
+        // through the canonical mapper so the field copy stays in lockstep with
-          id: restoredPage.id,
+        // buildTree. The server NULLS `temporaryExpiresAt` on restore (a restored
-          slugId: restoredPage.slugId,
+        // page is made permanent), so the mapper carries that null through and
+        // the node correctly shows no clock marker.
+        const nodeData: SpaceTreeNode = pageToTreeNode(restoredPage, {
           name: restoredPage.title || "Untitled",
-          icon: restoredPage.icon,
-          position: restoredPage.position,
-          spaceId: restoredPage.spaceId,
-          parentPageId: restoredPage.parentPageId,
           hasChildren: restoredPage.hasChildren || false,
-          children: [],
+        });
-        };
 
         // Determine the parent and index
         const parentId = restoredPage.parentPageId || null;

apps/client/src/features/page/tree/components/space-tree-node-menu.tsx

@@ -37,6 +37,7 @@ import {
 } from "@/features/page-embed/queries/page-embed-query";
 import { treeDataAtom } from "@/features/page/tree/atoms/tree-data-atom.ts";
 import { treeModel } from "@/features/page/tree/model/tree-model";
+import { pageToTreeNode } from "@/features/page/tree/utils";
 import { useTreeMutation } from "@/features/page/tree/hooks/use-tree-mutation.ts";
 import type { SpaceTreeNode } from "@/features/page/tree/types.ts";
 import classes from "@/features/page/tree/styles/tree.module.css";
@@ -130,18 +131,14 @@ export function NodeMenu({ node, canEdit }: NodeMenuProps) {
       const currentIndex = siblings?.index ?? 0;
       const newIndex = currentIndex + 1;
 
-      const treeNodeData: SpaceTreeNode = {
+      // Routed through the canonical mapper so the field copy stays in lockstep
-        id: duplicatedPage.id,
+      // with buildTree. The server does NOT arm a death timer on duplicate (the
-        slugId: duplicatedPage.slugId,
+      // copy's `temporaryExpiresAt` defaults to null = permanent), so the mapper
-        name: duplicatedPage.title,
+      // carries that null through and the duplicated node correctly shows no
-        position: duplicatedPage.position,
+      // clock marker — matching the server without a reload.
-        spaceId: duplicatedPage.spaceId,
+      const treeNodeData: SpaceTreeNode = pageToTreeNode(duplicatedPage, {
-        parentPageId: duplicatedPage.parentPageId,
-        icon: duplicatedPage.icon,
-        hasChildren: duplicatedPage.hasChildren,
         canEdit: true,
-        children: [],
+      });
-      };
 
       setData((prev) =>
         treeModel.insert(prev, parentId, treeNodeData, newIndex),

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

@@ -9,6 +9,7 @@ import { treeModel } from "@/features/page/tree/model/tree-model";
 import type { DropOp } from "@/features/page/tree/model/tree-model.types";
 import { dropOpToMovePayload } from "./drop-op-to-move-payload";
 import { SpaceTreeNode } from "@/features/page/tree/types.ts";
+import { pageToTreeNode } from "@/features/page/tree/utils";
 import { IPage } from "@/features/page/types/page.types.ts";
 import {
   useCreatePageMutation,
@@ -139,18 +140,15 @@ export function useTreeMutation(spaceId: string): UseTreeMutation {
         throw new Error("Failed to create page");
       }
 
-      const newNode: SpaceTreeNode = {
+      // Route through the canonical mapper so the field copy (esp.
-        id: createdPage.id,
+      // `temporaryExpiresAt`, which shows the temporary-note clock marker on
-        slugId: createdPage.slugId,
+      // optimistic insert) can't drift from buildTree. `name: ""` because a
+      // freshly created page is untitled; `hasChildren: false` because it has no
+      // children yet.
+      const newNode: SpaceTreeNode = pageToTreeNode(createdPage, {
         name: "",
-        position: createdPage.position,
-        spaceId: createdPage.spaceId,
-        parentPageId: createdPage.parentPageId,
         hasChildren: false,
-        // Show the temporary-note icon immediately on optimistic insert.
+      });
-        temporaryExpiresAt: createdPage.temporaryExpiresAt,
-        children: [],
-      };
 
       // Read latest tree at call time. Without this, callers that mutate the
       // tree (e.g. lazy-load children on expand) immediately before calling

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

@@ -393,6 +393,101 @@ describe("handleCreate optimistic-insert idempotency (find-then-skip)", () => {
   });
 });
 
+// handleCreate race-guard temporaryExpiresAt patch: when the server's
+// addTreeNode broadcast wins the race and inserts the node BEFORE the optimistic
+// updater runs, the updater must not re-insert. Two sub-branches:
+//  (a) the node the broadcast inserted carries NO deadline (an older broadcast
+//      omitted it) while the authoritative create response DOES → patch the
+//      deadline on so the clock marker shows now, without a reload.
+//  (b) the existing node ALREADY has a deadline → do NOT overwrite it; return
+//      `prev` by reference (a no-op write).
+describe("handleCreate race-guard temporaryExpiresAt patch", () => {
+  type TN = TreeNode<{ name: string; temporaryExpiresAt?: string | null }>;
+
+  // Mirrors the setData updater in use-tree-mutation handleCreate.
+  const applyOptimisticInsert = (
+    tree: TN[],
+    parentId: string | null,
+    node: TN,
+    index: number,
+  ): TN[] => {
+    const existing = treeModel.find(tree, node.id) as TN | null;
+    if (existing) {
+      if (node.temporaryExpiresAt && !existing.temporaryExpiresAt) {
+        return treeModel.update(tree, node.id, {
+          temporaryExpiresAt: node.temporaryExpiresAt,
+        });
+      }
+      return tree;
+    }
+    return treeModel.insert(tree, parentId, node, index);
+  };
+
+  const fixtureTN: TN[] = [
+    { id: "a", name: "A" },
+    { id: "b", name: "B" },
+  ];
+
+  const deadline = "2026-07-01T00:00:00.000Z";
+
+  it("(a) patches temporaryExpiresAt when the existing node has none + the response carries a deadline", () => {
+    // Server broadcast won the race and inserted the node WITHOUT a deadline.
+    const afterServer = treeModel.insert(fixtureTN, null, {
+      id: "new",
+      name: "",
+    });
+    expect((treeModel.find(afterServer, "new") as TN).temporaryExpiresAt).toBe(
+      undefined,
+    );
+
+    // The authoritative create response carries the deadline.
+    const created: TN = { id: "new", name: "", temporaryExpiresAt: deadline };
+    const patched = applyOptimisticInsert(
+      afterServer,
+      null,
+      created,
+      afterServer.length,
+    );
+
+    // A new reference (the patch wrote) and the node now has the deadline...
+    expect(patched).not.toBe(afterServer);
+    expect((treeModel.find(patched, "new") as TN).temporaryExpiresAt).toBe(
+      deadline,
+    );
+    // ...and still exactly one node (no duplicate re-insert).
+    expect(patched.filter((n) => n.id === "new")).toHaveLength(1);
+  });
+
+  it("(b) does NOT overwrite an existing deadline; returns prev by reference", () => {
+    const existingDeadline = deadline;
+    // The node already exists WITH a deadline (the broadcast carried it).
+    const afterServer = treeModel.insert(fixtureTN, null, {
+      id: "new",
+      name: "",
+      temporaryExpiresAt: existingDeadline,
+    });
+
+    // The create response carries a DIFFERENT deadline; the guard must ignore it.
+    const created: TN = {
+      id: "new",
+      name: "",
+      temporaryExpiresAt: "2099-01-01T00:00:00.000Z",
+    };
+    const after = applyOptimisticInsert(
+      afterServer,
+      null,
+      created,
+      afterServer.length,
+    );
+
+    // prev returned by reference (no write) and the original deadline is kept.
+    expect(after).toBe(afterServer);
+    expect((treeModel.find(after, "new") as TN).temporaryExpiresAt).toBe(
+      existingDeadline,
+    );
+  });
+});
+
 // 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

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

@@ -9,26 +9,45 @@ export function sortPositionKeys(keys: any[]) {
   });
 }
 
+/**
+ * Single canonical `IPage -> SpaceTreeNode` field mapper. Every place that
+ * materialises a tree node from a page (buildTree, the optimistic insert in
+ * handleCreate, restore, duplicate) routes through here so the field copy —
+ * crucially `temporaryExpiresAt` — can never silently drift between sites. The
+ * `overrides` cover the small per-site differences (e.g. `name: ""` for an
+ * optimistic create, `name: title || "Untitled"` for restore, `canEdit: true`
+ * for duplicate). The default `temporaryExpiresAt` comes straight off the page,
+ * so restore (which the server nulls) stays permanent and a temporary create
+ * keeps its clock marker without a reload.
+ */
+export function pageToTreeNode(
+  page: IPage,
+  overrides?: Partial<SpaceTreeNode>,
+): SpaceTreeNode {
+  return {
+    id: page.id,
+    slugId: page.slugId,
+    name: page.title,
+    icon: page.icon,
+    position: page.position,
+    hasChildren: page.hasChildren,
+    spaceId: page.spaceId,
+    parentPageId: page.parentPageId,
+    canEdit: page.canEdit ?? page.permissions?.canEdit,
+    isTemplate: page.isTemplate,
+    temporaryExpiresAt: page.temporaryExpiresAt,
+    children: [],
+    ...overrides,
+  };
+}
+
 export function buildTree(pages: IPage[]): SpaceTreeNode[] {
   const pageMap: Record<string, SpaceTreeNode> = {};
 
   const tree: SpaceTreeNode[] = [];
 
   pages.forEach((page) => {
-    pageMap[page.id] = {
+    pageMap[page.id] = pageToTreeNode(page);
-      id: page.id,
-      slugId: page.slugId,
-      name: page.title,
-      icon: page.icon,
-      position: page.position,
-      hasChildren: page.hasChildren,
-      spaceId: page.spaceId,
-      parentPageId: page.parentPageId,
-      canEdit: page.canEdit ?? page.permissions?.canEdit,
-      isTemplate: page.isTemplate,
-      temporaryExpiresAt: page.temporaryExpiresAt,
-      children: [],
-    };
   });
 
   // Defense-in-depth: a duplicate id in `pages` would push two references to the

apps/server/src/database/listeners/page.listener.ts

@@ -28,6 +28,36 @@ export interface TreeNodeSnapshot {
   temporaryExpiresAt?: Date | string | null;
 }
 
+/**
+ * Single canonical builder for a `TreeNodeSnapshot` from a page-like row. Both
+ * the `PAGE_CREATED` event enrichment (`page.repo.insertPage`) and the
+ * `addTreeNode` broadcast (`WsTreeService.broadcastPageCreated`) build this same
+ * snapshot; routing both through here keeps the optional `temporaryExpiresAt`
+ * (and the `?? null` normalisation that pins a permanent note to an explicit
+ * null) from silently drifting between the two literals.
+ */
+export function toTreeNodeSnapshot(page: {
+  id: string;
+  slugId: string;
+  title: string | null;
+  icon: string | null;
+  position: string;
+  spaceId: string;
+  parentPageId: string | null;
+  temporaryExpiresAt?: Date | string | null;
+}): TreeNodeSnapshot {
+  return {
+    id: page.id,
+    slugId: page.slugId,
+    title: page.title,
+    icon: page.icon,
+    position: page.position,
+    spaceId: page.spaceId,
+    parentPageId: page.parentPageId,
+    temporaryExpiresAt: page.temporaryExpiresAt ?? null,
+  };
+}
+
 export class PageEvent {
   pageIds: string[];
   workspaceId: string;

apps/server/src/database/repos/page/page.repo.ts

@@ -16,7 +16,10 @@ import { jsonArrayFrom, jsonObjectFrom } from 'kysely/helpers/postgres';
 import { SpaceMemberRepo } from '@docmost/db/repos/space/space-member.repo';
 import { EventEmitter2 } from '@nestjs/event-emitter';
 import { EventName } from '../../../common/events/event.contants';
-import { TreeUpdateSnapshot } from '../../listeners/page.listener';
+import {
+  TreeUpdateSnapshot,
+  toTreeNodeSnapshot,
+} from '../../listeners/page.listener';
 
 /**
  * Optional extras for the PAGE_UPDATED event emitted by updatePage(s). Lets the
@@ -200,20 +203,10 @@ export class PageRepo {
     this.eventEmitter.emit(EventName.PAGE_CREATED, {
       pageIds: [result.id],
       workspaceId: result.workspaceId,
-      pages: [
+      // Built via the shared snapshot helper so the field copy (and the
-        {
+      // death-timer deadline that shows the sidebar clock marker without a
-          id: result.id,
+      // reload) can't drift from the `addTreeNode` broadcast literal.
-          slugId: result.slugId,
+      pages: [toTreeNodeSnapshot(result)],
-          title: result.title,
-          icon: result.icon,
-          position: result.position,
-          spaceId: result.spaceId,
-          parentPageId: result.parentPageId,
-          // Carry the death-timer deadline so a note created as temporary shows
-          // its sidebar clock marker on every client without a reload.
-          temporaryExpiresAt: result.temporaryExpiresAt,
-        },
-      ],
     });
 
     return result;

apps/server/src/ws/ws-tree.service.spec.ts

@@ -83,6 +83,27 @@ describe('WsTreeService', () => {
     );
   });
 
+  it('broadcastPageCreated carries temporaryExpiresAt when the page is a temporary note', async () => {
+    const expiresAt = new Date('2026-07-01T00:00:00.000Z');
+    await service.broadcastPageCreated({ ...snapshot, temporaryExpiresAt: expiresAt });
+
+    const data =
+      wsService.emitTreeEvent.mock.calls[0][2].payload.data;
+    // The death-timer deadline reaches receivers so the clock marker renders
+    // immediately (incl. the author if this broadcast wins the optimistic race).
+    expect(data.temporaryExpiresAt).toBe(expiresAt);
+  });
+
+  it('broadcastPageCreated pins temporaryExpiresAt to null for a permanent page', async () => {
+    // Fixture omits temporaryExpiresAt; the `?? null` must send an explicit null
+    // (permanent) rather than undefined, so receivers clear any stale marker.
+    await service.broadcastPageCreated(snapshot);
+
+    const data =
+      wsService.emitTreeEvent.mock.calls[0][2].payload.data;
+    expect(data.temporaryExpiresAt).toBeNull();
+  });
+
   it('broadcastPageDeleted emits deleteTreeNode with the root node only', async () => {
     await service.broadcastPageDeleted({
       ...snapshot,

apps/server/src/ws/ws-tree.service.ts

@@ -5,6 +5,7 @@ import {
   PageMovedEvent,
   TreeNodeSnapshot,
   TreeUpdateSnapshot,
+  toTreeNodeSnapshot,
 } from '../database/listeners/page.listener';
 
 @Injectable()
@@ -28,20 +29,17 @@ export class WsTreeService {
         // Receivers place by `position` among already-loaded siblings, not by
         // this absolute index (sender's loaded set differs from receivers').
         index: 0,
+        // Built via the shared snapshot helper (same one page.repo uses to fill
+        // the event), then extended with the tree-only fields the client
+        // receiver consumes. The helper carries the death-timer deadline
+        // (normalised to null => permanent) so receivers — and the author, if
+        // this broadcast wins the race against the optimistic insert — render
+        // the temporary-note clock marker immediately, without it drifting from
+        // the event literal.
         data: {
-          id: page.id,
+          ...toTreeNodeSnapshot(page),
-          slugId: page.slugId,
           name: page.title ?? '',
-          title: page.title,
-          icon: page.icon,
-          position: page.position,
-          spaceId: page.spaceId,
-          parentPageId: page.parentPageId,
           hasChildren: false,
-          // Carry the death-timer deadline so receivers (and the author, if this
-          // broadcast wins the race against the optimistic insert) render the
-          // temporary-note clock marker immediately. null => permanent.
-          temporaryExpiresAt: page.temporaryExpiresAt ?? null,
           children: [],
         },
       },