Merge remote-tracking branch 'gitea/develop' into HEAD

# Conflicts:
#	apps/server/src/collaboration/extensions/persistence-store.spec.ts
#	apps/server/src/collaboration/extensions/persistence.extension.ts

CHANGELOG.md

@@ -67,11 +67,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   `nosniff` + restrictive CSP + attachment disposition for non-image mimes) and
   are RAM-only, bound to the instance that created them. Tunable via five
   `SANDBOX_*` env vars (see `.env.example`). (#243)
-- **Inline spoiler mark — hide text behind click-to-reveal blur.** Selected text
-  can be marked as a spoiler from a new bubble-menu toggle, or typed Discord-style
-  with the `||text||` input rule; the rendered span blurs until clicked to reveal.
-  The mark is preserved losslessly through Markdown export/import (as a raw
-  `<span data-spoiler="true">…</span>`) and on public shares. (#259)
 
 ### Changed
 
@@ -129,6 +124,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   "This address is in use. Saving will move it to this page." — and keeps Save
   enabled, so the existing reassign-confirm flow (`409 ALIAS_REASSIGN_REQUIRED` →
   "Move custom address?") is discoverable instead of reading as terminal. (#227)
+- **A non-empty page can no longer be silently lost to a momentarily-empty live
+  document.** The server's persistence guard now refuses to overwrite non-empty
+  persisted content with an empty live Y.Doc — a transient emptiness from a
+  glitch, a bad merge, or an emptying transclusion no longer wipes the saved
+  page. A *deliberate* clear still works: a select-all + Delete in the editor
+  emits a single-use "intentional clear" signal that lets exactly that one empty
+  write through the guard, so genuinely emptying a page is persisted while
+  accidental empties are blocked. (#248, #251)
 
 ### Security
 

apps/client/public/locales/en-US/translation.json

@@ -352,7 +352,6 @@
   "Underline": "Underline",
   "Strike": "Strike",
   "Code": "Code",
-  "Spoiler": "Spoiler",
   "Comment": "Comment",
   "Text": "Text",
   "Heading 1": "Heading 1",

apps/client/public/locales/ru-RU/translation.json

@@ -351,7 +351,6 @@
   "Underline": "Подчёркнутый",
   "Strike": "Перечёркнутый",
   "Code": "Код",
-  "Spoiler": "Спойлер",
   "Comment": "Комментарий",
   "Text": "Текст",
   "Heading 1": "Заголовок 1",

apps/client/src/features/editor/components/bubble-menu/bubble-menu.tsx

@@ -9,7 +9,6 @@ import {
   IconStrikethrough,
   IconUnderline,
   IconMessage,
-  IconEyeOff,
 } from "@tabler/icons-react";
 import clsx from "clsx";
 import classes from "./bubble-menu.module.css";
@@ -75,7 +74,6 @@ export const EditorBubbleMenu: FC<EditorBubbleMenuProps> = (props) => {
         isStrike: ctx.editor.isActive("strike"),
         isCode: ctx.editor.isActive("code"),
         isComment: ctx.editor.isActive("comment"),
-        isSpoiler: ctx.editor.isActive("spoiler"),
       };
     },
   });
@@ -111,12 +109,6 @@ export const EditorBubbleMenu: FC<EditorBubbleMenuProps> = (props) => {
       command: () => props.editor.chain().focus().toggleCode().run(),
       icon: IconCode,
     },
-    {
-      name: "Spoiler",
-      isActive: () => editorState?.isSpoiler,
-      command: () => props.editor.chain().focus().toggleSpoiler().run(),
-      icon: IconEyeOff,
-    },
   ];
 
   const commentItem: BubbleMenuItem = {

apps/client/src/features/editor/components/spoiler/spoiler-view.tsx

@@ -1,20 +0,0 @@
-import { MarkViewContent, MarkViewProps } from "@tiptap/react";
-import { useState } from "react";
-
-// Click-to-reveal spoiler. The revealed state is UI-only and is never written to
-// the document: toggling only adds/removes the `is-revealed` class (CSS removes
-// the blur). renderHTML never emits `is-revealed`, so it can't leak into the
-// doc/clipboard. Works the same in editor, read-only and public-share views.
-export default function SpoilerView(_props: MarkViewProps) {
-  const [revealed, setRevealed] = useState(false);
-
-  return (
-    <span
-      className={revealed ? "spoiler is-revealed" : "spoiler"}
-      data-spoiler="true"
-      onClick={() => setRevealed((v) => !v)}
-    >
-      <MarkViewContent />
-    </span>
-  );
-}

apps/client/src/features/editor/extensions/extensions.ts

@@ -53,7 +53,6 @@ import {
   Subpages,
   Heading,
   Highlight,
-  Spoiler,
   Indent,
   UniqueID,
   SharedStorage,
@@ -117,7 +116,6 @@ import mentionRenderItems from "@/features/editor/components/mention/mention-sug
 import { ReactNodeViewRenderer, ReactMarkViewRenderer } from "@tiptap/react";
 import MentionView from "@/features/editor/components/mention/mention-view.tsx";
 import LinkView from "@/features/editor/components/link/link-view.tsx";
-import SpoilerView from "@/features/editor/components/spoiler/spoiler-view.tsx";
 import i18n from "@/i18n.ts";
 import { MarkdownClipboard } from "@/features/editor/extensions/markdown-clipboard.ts";
 import EmojiCommand from "./emoji-command";
@@ -125,6 +123,7 @@ import { countWords } from "alfaaz";
 import AutoJoiner from "@/features/editor/extensions/autojoiner.ts";
 import GlobalDragHandle from "@/features/editor/extensions/drag-handle.ts";
 import { CleanStyles } from "@/features/editor/extensions/clean-styles.ts";
+import { IntentionalClear } from "@/features/editor/extensions/intentional-clear.ts";
 
 const lowlight = createLowlight(common);
 lowlight.register("mermaid", plaintext);
@@ -239,11 +238,6 @@ export const mainExtensions = [
   Highlight.configure({
     multicolor: true,
   }),
-  Spoiler.configure({}).extend({
-    addMarkView() {
-      return ReactMarkViewRenderer(SpoilerView);
-    },
-  }),
   Typography,
   TrailingNode,
   GlobalDragHandle.configure({
@@ -493,4 +487,10 @@ export const collabExtensions: CollabExtensions = (provider, user) => [
       color: randomElement(userColors),
     },
   }),
+  // #251 — emit an intentional-clear signal to the server when the user
+  // deliberately empties the page, so the #248 store-side empty-guard lets that
+  // one clear through while still blocking accidental empties.
+  IntentionalClear.configure({
+    provider,
+  }),
 ];

apps/client/src/features/editor/extensions/intentional-clear.test.ts

@@ -0,0 +1,120 @@
+import { describe, it, expect, vi, beforeEach } from "vitest";
+import { Editor } from "@tiptap/core";
+import { Document } from "@tiptap/extension-document";
+import { Paragraph } from "@tiptap/extension-paragraph";
+import { Text } from "@tiptap/extension-text";
+import { ySyncPluginKey } from "@tiptap/y-tiptap";
+import {
+  IntentionalClear,
+  INTENTIONAL_CLEAR_MESSAGE_TYPE,
+} from "./intentional-clear";
+
+/**
+ * #251 — the intentional-clear signal is driven through the REAL editor path:
+ * a fresh Editor with the IntentionalClear extension, a fake provider that
+ * records sendStateless, and the actual select-all + delete command the user's
+ * keystroke runs. No hand-poke of any flag.
+ */
+describe("IntentionalClear extension", () => {
+  let sendStateless: ReturnType<typeof vi.fn>;
+
+  const makeEditor = (content: unknown) =>
+    new Editor({
+      extensions: [
+        Document,
+        Paragraph,
+        Text,
+        IntentionalClear.configure({
+          // Minimal provider stand-in: only sendStateless is exercised.
+          provider: { sendStateless } as any,
+        }),
+      ],
+      content: content as any,
+    });
+
+  beforeEach(() => {
+    sendStateless = vi.fn();
+  });
+
+  it("emits the clear signal when a user empties a non-empty doc (select-all + delete)", () => {
+    const editor = makeEditor({
+      type: "doc",
+      content: [
+        { type: "paragraph", content: [{ type: "text", text: "hello world" }] },
+      ],
+    });
+
+    // The exact command path a select-all + Delete keystroke dispatches.
+    editor.chain().selectAll().deleteSelection().run();
+
+    expect(sendStateless).toHaveBeenCalledTimes(1);
+    const payload = JSON.parse(sendStateless.mock.calls[0][0]);
+    expect(payload).toEqual({ type: INTENTIONAL_CLEAR_MESSAGE_TYPE });
+
+    editor.destroy();
+  });
+
+  it("does NOT emit when typing into an empty doc (no non-empty → empty transition)", () => {
+    const editor = makeEditor({ type: "doc", content: [{ type: "paragraph" }] });
+
+    editor.chain().insertContent("typed text").run();
+
+    expect(sendStateless).not.toHaveBeenCalled();
+    editor.destroy();
+  });
+
+  it("does NOT emit on an edit that leaves the doc non-empty", () => {
+    const editor = makeEditor({
+      type: "doc",
+      content: [
+        { type: "paragraph", content: [{ type: "text", text: "keep me" }] },
+      ],
+    });
+
+    editor.chain().insertContent(" more").run();
+
+    expect(sendStateless).not.toHaveBeenCalled();
+    editor.destroy();
+  });
+
+  it("does NOT emit when a REMOTE/merge (change-origin) transaction empties the doc", () => {
+    // This pins the CENTRAL #248 protection: only a LOCAL user edit may emit the
+    // intentional-clear signal. An emptiness arriving from another client, a bad
+    // merge, or an emptied transclusion is applied as a y-sync transaction tagged
+    // with the ySyncPluginKey meta, which `isChangeOrigin` detects. The extension
+    // must early-return on it and NOT punch the empty write through the server
+    // guard.
+    const editor = makeEditor({
+      type: "doc",
+      content: [
+        { type: "paragraph", content: [{ type: "text", text: "remote content" }] },
+      ],
+    });
+
+    // Build a transaction that empties the non-empty doc and tag it exactly the
+    // way y-tiptap tags a remote y-sync update: `tr.setMeta(ySyncPluginKey,
+    // { isChangeOrigin: true })` (see @tiptap/y-tiptap sync-plugin). This makes
+    // the real `isChangeOrigin(tr)` predicate return true — not a stand-in.
+    const { state } = editor;
+    const tr = state.tr
+      .delete(0, state.doc.content.size)
+      .setMeta(ySyncPluginKey, { isChangeOrigin: true });
+    editor.view.dispatch(tr);
+
+    // The transaction really emptied the doc (became the single empty paragraph)…
+    expect(editor.state.doc.textContent).toBe("");
+    // …yet because it is change-origin, no signal is emitted.
+    expect(sendStateless).not.toHaveBeenCalled();
+    editor.destroy();
+  });
+
+  it("does NOT emit when the doc was already empty", () => {
+    const editor = makeEditor({ type: "doc", content: [{ type: "paragraph" }] });
+
+    // Selecting all + delete on an already-empty doc is a no-op transition.
+    editor.chain().selectAll().deleteSelection().run();
+
+    expect(sendStateless).not.toHaveBeenCalled();
+    editor.destroy();
+  });
+});

apps/client/src/features/editor/extensions/intentional-clear.ts

@@ -0,0 +1,94 @@
+import { Extension } from "@tiptap/core";
+import { isChangeOrigin } from "@tiptap/extension-collaboration";
+import type { Node as PMNode } from "@tiptap/pm/model";
+import type { HocuspocusProvider } from "@hocuspocus/provider";
+
+/**
+ * Stateless message type sent to the server when a user deliberately clears a
+ * page to empty. Kept in one place so the client emitter and the server
+ * consumer (PersistenceExtension.onStateless) agree on the wire format.
+ */
+export const INTENTIONAL_CLEAR_MESSAGE_TYPE = "intentional-clear";
+
+export interface IntentionalClearOptions {
+  /** The collab provider used to send the stateless clear signal. */
+  provider: HocuspocusProvider | null;
+}
+
+/**
+ * A "document is empty" check that mirrors the server's `isEmptyParagraphDoc`
+ * (collaboration.util.ts): exactly one top-level paragraph with no inline
+ * content. After a select-all + delete TipTap leaves precisely this shape, so
+ * matching it here keeps the client signal aligned with the server guard that
+ * consumes it.
+ */
+function isEmptyParagraphDoc(doc: PMNode): boolean {
+  if (doc.childCount !== 1) return false;
+  const child = doc.firstChild;
+  return (
+    child !== null &&
+    child !== undefined &&
+    child.type.name === "paragraph" &&
+    child.content.size === 0
+  );
+}
+
+/**
+ * #251 — intentional-clear signal.
+ *
+ * The server's #248 store-side empty-guard unconditionally refuses to overwrite
+ * non-empty persisted content with an empty document, because a momentarily
+ * empty live Y.Doc (a glitch, a bad merge, an emptying transclusion) is
+ * indistinguishable from a real clear *at the store layer*. That protection is
+ * correct, but it also blocks a user who genuinely wants to empty the page.
+ *
+ * This extension supplies the missing distinction. It watches LOCAL, user-driven
+ * transactions and, the moment one reduces a non-empty document to the empty
+ * single-paragraph shape, it sends a hocuspocus stateless message to the server.
+ * The server records a short-lived, single-use "intentional clear pending" flag
+ * for this document that the next (debounced) onStoreDocument consumes to let
+ * that one empty write through the guard.
+ *
+ * What counts as an intentional clear (precise definition):
+ *  - the transaction actually changed the document (`docChanged`), AND
+ *  - it is a LOCAL user edit, not a remote collab application — remote y-sync
+ *    transactions are tagged and filtered out via `isChangeOrigin`, so an
+ *    emptiness that arrives from another client / a merge never emits a signal,
+ *    AND
+ *  - the document was non-empty before the transaction and is the empty
+ *    single-paragraph doc after it.
+ *
+ * This is exactly the select-all + Delete / Backspace (or any local command that
+ * empties the doc, e.g. clearContent) keystroke path. A transient/programmatic
+ * empty serialization that the server might see on the wire does NOT come with
+ * this signal, so the guard still blocks it.
+ */
+export const IntentionalClear = Extension.create<IntentionalClearOptions>({
+  name: "intentionalClear",
+
+  addOptions() {
+    return {
+      provider: null,
+    };
+  },
+
+  onTransaction({ transaction }) {
+    if (!transaction.docChanged) return;
+    // Only react to local user edits. Remote collaboration steps (and other
+    // y-sync-applied changes) carry the change origin and must never be treated
+    // as an intentional clear, otherwise a remote/merge-induced emptiness would
+    // punch through the server guard.
+    if (isChangeOrigin(transaction)) return;
+
+    const becameEmpty =
+      !isEmptyParagraphDoc(transaction.before) &&
+      isEmptyParagraphDoc(transaction.doc);
+    if (!becameEmpty) return;
+
+    // The server reads the originating document from the connection, so the
+    // payload only needs to declare intent — it cannot target another document.
+    this.options.provider?.sendStateless(
+      JSON.stringify({ type: INTENTIONAL_CLEAR_MESSAGE_TYPE }),
+    );
+  },
+});

apps/client/src/features/editor/styles/index.css

@@ -14,7 +14,6 @@
 @import "./mention.css";
 @import "./ordered-list.css";
 @import "./highlight.css";
-@import "./spoiler.css";
 @import "./indent.css";
 @import "./columns.css";
 @import "./status.css";

apps/client/src/features/editor/styles/spoiler.css

@@ -1,21 +0,0 @@
-.spoiler {
-  background: rgba(0, 0, 0, 0.85);
-  border-radius: 0.25em;
-  cursor: pointer;
-  filter: blur(0.3em);
-  transition: filter 0.15s ease;
-  user-select: none;
-}
-
-.spoiler.is-revealed {
-  filter: none;
-  background: rgba(125, 125, 125, 0.18);
-  user-select: auto;
-}
-
-@media print {
-  .spoiler {
-    filter: none;
-    background: rgba(125, 125, 125, 0.18);
-  }
-}

apps/client/src/features/notification/notification.utils.test.ts

@@ -1,5 +1,7 @@
 import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
+import i18n from "@/i18n.ts";
 import {
+  formatRelativeTime,
   getTimeGroup,
   groupNotificationsByTime,
 } from "@/features/notification/notification.utils.ts";
@@ -132,3 +134,59 @@ describe("groupNotificationsByTime", () => {
     expect(groupNotificationsByTime([], labels)).toEqual([]);
   });
 });
+
+describe("formatRelativeTime — relative buckets and absolute-date fallback", () => {
+  // Distinct fixed clock for the relative formatter (uses Date.now via `new
+  // Date()`), so the bucket boundaries are deterministic under fake timers.
+  const NOW = new Date("2026-06-15T12:00:00.000Z");
+  const MIN = 60_000;
+
+  beforeEach(() => {
+    vi.setSystemTime(NOW);
+  });
+
+  // ISO string `ms` milliseconds before NOW.
+  function ago(ms: number): string {
+    return new Date(NOW.getTime() - ms).toISOString();
+  }
+
+  it("returns the i18n 'now' label for anything under a minute", () => {
+    expect(formatRelativeTime(ago(0))).toBe(i18n.t("now"));
+    expect(formatRelativeTime(ago(59_000))).toBe(i18n.t("now"));
+  });
+
+  it("crosses into the minutes bucket exactly at 1 minute", () => {
+    expect(formatRelativeTime(ago(MIN - 1000))).toBe(i18n.t("now"));
+    expect(formatRelativeTime(ago(MIN))).toBe("1m");
+    expect(formatRelativeTime(ago(5 * MIN))).toBe("5m");
+    expect(formatRelativeTime(ago(59 * MIN))).toBe("59m");
+  });
+
+  it("crosses into the hours bucket exactly at 60 minutes", () => {
+    expect(formatRelativeTime(ago(60 * MIN - 1000))).toBe("59m");
+    expect(formatRelativeTime(ago(HOUR))).toBe("1h");
+    expect(formatRelativeTime(ago(23 * HOUR))).toBe("23h");
+  });
+
+  it("crosses into the days bucket exactly at 24 hours", () => {
+    expect(formatRelativeTime(ago(24 * HOUR - 1000))).toBe("23h");
+    expect(formatRelativeTime(ago(DAY))).toBe("1d");
+    expect(formatRelativeTime(ago(6 * DAY))).toBe("6d");
+  });
+
+  it("falls back to an absolute short date once >= 7 days old", () => {
+    // 6d -> still relative; 7d -> absolute date (no longer N[mhd], and equal to
+    // the localized short-date of the source timestamp).
+    expect(formatRelativeTime(ago(6 * DAY))).toBe("6d");
+
+    const sevenDaysAgo = ago(7 * DAY);
+    const result = formatRelativeTime(sevenDaysAgo);
+    expect(result).not.toMatch(/^\d+[mhd]$/);
+    expect(result).not.toBe(i18n.t("now"));
+    const expected = new Intl.DateTimeFormat(i18n.language, {
+      month: "short",
+      day: "numeric",
+    }).format(new Date(sevenDaysAgo));
+    expect(result).toBe(expected);
+  });
+});

apps/client/src/features/page/tree/utils/find-breadcrumb-path.test.ts

@@ -0,0 +1,79 @@
+import { describe, it, expect } from "vitest";
+import { findBreadcrumbPath } from "./utils";
+import type { SpaceTreeNode } from "@/features/page/tree/types.ts";
+
+// findBreadcrumbPath walks the live, SHARED sidebar tree. The high-value
+// invariant: when a node has no usable name it must surface "Untitled" ONLY on
+// the returned breadcrumb chain via a shallow copy — never by mutating the input
+// node (which would silently rename the node in the sidebar). Also covers normal
+// ancestor-chain resolution, the not-found case, and nested children.
+
+function node(id: string, over: Partial<SpaceTreeNode> = {}): SpaceTreeNode {
+  return {
+    id,
+    slugId: `slug-${id}`,
+    name: id.toUpperCase(),
+    icon: undefined,
+    position: "a0",
+    spaceId: "space-1",
+    parentPageId: null as unknown as string,
+    hasChildren: false,
+    children: [],
+    ...over,
+  };
+}
+
+describe("findBreadcrumbPath", () => {
+  it("does NOT mutate the input tree when a node has an empty/whitespace name", () => {
+    // A whitespace-only-named node nested under a blank-named root.
+    const target = node("target", { name: "   " });
+    const root = node("root", { name: "", hasChildren: true, children: [target] });
+    const tree = [root];
+
+    const result = findBreadcrumbPath(tree, "target");
+
+    expect(result).not.toBeNull();
+    // The RETURNED chain shows "Untitled" for both blank nodes.
+    expect(result!.map((n) => n.name)).toEqual(["Untitled", "Untitled"]);
+    // The original input nodes are untouched (still blank).
+    expect(root.name).toBe("");
+    expect(target.name).toBe("   ");
+    // The renamed breadcrumb entries are fresh copies, not the input objects.
+    expect(result![0]).not.toBe(root);
+    expect(result![1]).not.toBe(target);
+  });
+
+  it("returns the SAME node reference (no copy) when the name is non-empty", () => {
+    // No rename needed -> the node is passed through by reference (cheap path).
+    const target = node("target", { name: "Real Title" });
+    const result = findBreadcrumbPath([target], "target");
+    expect(result![0]).toBe(target);
+    expect(result![0].name).toBe("Real Title");
+  });
+
+  it("resolves the full ancestor chain ending at the target", () => {
+    const target = node("c");
+    const mid = node("b", { hasChildren: true, children: [target] });
+    const root = node("a", { hasChildren: true, children: [mid] });
+    const result = findBreadcrumbPath([root], "c");
+    expect(result!.map((n) => n.id)).toEqual(["a", "b", "c"]);
+  });
+
+  it("finds a target nested under a deeper sibling branch", () => {
+    // Two root branches; the target lives inside the second branch's child.
+    const target = node("deep");
+    const branch2 = node("r2", {
+      hasChildren: true,
+      children: [node("x"), node("y", { hasChildren: true, children: [target] })],
+    });
+    const branch1 = node("r1", { hasChildren: true, children: [node("z")] });
+    const result = findBreadcrumbPath([branch1, branch2], "deep");
+    expect(result!.map((n) => n.id)).toEqual(["r2", "y", "deep"]);
+  });
+
+  it("returns null when the page id is not present in the tree", () => {
+    const root = node("root", { hasChildren: true, children: [node("child")] });
+    expect(findBreadcrumbPath([root], "missing")).toBeNull();
+    expect(findBreadcrumbPath([], "anything")).toBeNull();
+  });
+});

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

@@ -8,6 +8,8 @@ import {
   closeIds,
   mergeRootTrees,
   loadedOpenBranchIds,
+  sortPositionKeys,
+  pageToTreeNode,
 } from "./utils";
 import type { IPage } from "@/features/page/types/page.types.ts";
 import type { SpaceTreeNode } from "@/features/page/tree/types.ts";
@@ -60,6 +62,82 @@ function treeNode(id: string, children: SpaceTreeNode[] = []): SpaceTreeNode {
   };
 }
 
+describe("sortPositionKeys", () => {
+  it("orders items ascending by their fractional `position` string", () => {
+    const items = [
+      { id: "c", position: "a5" },
+      { id: "a", position: "a1" },
+      { id: "b", position: "a3" },
+    ];
+    expect(sortPositionKeys(items).map((i) => i.id)).toEqual(["a", "b", "c"]);
+  });
+
+  it("is a stable sort: equal positions keep their input order", () => {
+    const items = [
+      { id: "x", position: "a1" },
+      { id: "y", position: "a1" },
+      { id: "z", position: "a1" },
+    ];
+    expect(sortPositionKeys(items).map((i) => i.id)).toEqual(["x", "y", "z"]);
+  });
+});
+
+describe("pageToTreeNode", () => {
+  function pageRow(over: Partial<IPage> = {}): IPage {
+    return {
+      id: "p1",
+      slugId: "slug-p1",
+      title: "My Page",
+      icon: "📄",
+      position: "a1",
+      hasChildren: true,
+      spaceId: "space-1",
+      parentPageId: null as unknown as string,
+      ...over,
+    } as IPage;
+  }
+
+  it("maps page.title -> node.name and copies the core fields", () => {
+    const node = pageToTreeNode(pageRow());
+    // The non-trivial transform: a page's `title` becomes the tree node's `name`.
+    expect(node.name).toBe("My Page");
+    expect(node.id).toBe("p1");
+    expect(node.slugId).toBe("slug-p1");
+    expect(node.icon).toBe("📄");
+    expect(node.position).toBe("a1");
+    expect(node.spaceId).toBe("space-1");
+    expect(node.hasChildren).toBe(true);
+    // Always materialized with an empty children array.
+    expect(node.children).toEqual([]);
+  });
+
+  it("derives canEdit from page.permissions.canEdit when the flat field is absent", () => {
+    const node = pageToTreeNode(
+      pageRow({ canEdit: undefined, permissions: { canEdit: true } } as Partial<IPage>),
+    );
+    expect(node.canEdit).toBe(true);
+  });
+
+  it("prefers the flat page.canEdit over permissions.canEdit", () => {
+    const node = pageToTreeNode(
+      pageRow({ canEdit: false, permissions: { canEdit: true } } as Partial<IPage>),
+    );
+    expect(node.canEdit).toBe(false);
+  });
+
+  it("carries temporaryExpiresAt straight off the page", () => {
+    const expiresAt = "2026-06-27T21:00:00.000Z";
+    expect(pageToTreeNode(pageRow({ temporaryExpiresAt: expiresAt })).temporaryExpiresAt).toBe(
+      expiresAt,
+    );
+  });
+
+  it("applies overrides on top of the mapped fields (e.g. optimistic blank name)", () => {
+    const node = pageToTreeNode(pageRow(), { name: "" });
+    expect(node.name).toBe("");
+  });
+});
+
 describe("buildTree", () => {
   it("builds one node per unique page", () => {
     const tree = buildTree([page("a", "a1"), page("b", "a2")]);

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

@@ -70,18 +70,22 @@ export function findBreadcrumbPath(
   path: SpaceTreeNode[] = [],
 ): SpaceTreeNode[] | null {
   for (const node of tree) {
-    if (!node.name || node.name.trim() === "") {
-      node.name = "Untitled";
-    }
+    // Never mutate the input tree (it is the live, shared sidebar tree state).
+    // When a node has no usable name, surface "Untitled" via a shallow copy that
+    // only the returned breadcrumb chain sees — the source node stays untouched.
+    const displayNode: SpaceTreeNode =
+      !node.name || node.name.trim() === ""
+        ? { ...node, name: "Untitled" }
+        : node;
 
     if (node.id === pageId) {
-      return [...path, node];
+      return [...path, displayNode];
     }
 
     if (node.children) {
       const newPath = findBreadcrumbPath(node.children, pageId, [
         ...path,
-        node,
+        displayNode,
       ]);
       if (newPath) {
         return newPath;

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

@@ -3,6 +3,7 @@ import {
   applyAddTreeNode,
   applyMoveTreeNode,
   applyDeleteTreeNode,
+  applyUpdateOne,
 } from "./tree-socket-reducers";
 import { treeModel } from "@/features/page/tree/model/tree-model";
 import { SpaceTreeNode } from "@/features/page/tree/types.ts";
@@ -338,3 +339,76 @@ describe("applyAddTreeNode", () => {
     expect(treeModel.find(next, "temp")?.temporaryExpiresAt).toBe(expiresAt);
   });
 });
+
+describe("applyUpdateOne", () => {
+  // A loaded two-level tree so we can patch both a root and a nested node.
+  const buildTree = (): SpaceTreeNode[] => [
+    node("root", {
+      position: "a0",
+      name: "Root",
+      icon: "📁",
+      hasChildren: true,
+      children: [node("child", { position: "a1", parentPageId: "root", name: "Child", icon: "📄" })],
+    }),
+  ];
+
+  // Build the UpdateEvent envelope; only `id`/`payload` matter to the reducer.
+  const ev = (id: string, payload: Record<string, unknown>) =>
+    ({
+      operation: "updateOne",
+      spaceId: "space-1",
+      entity: ["pages"],
+      id,
+      payload,
+    }) as unknown as Parameters<typeof applyUpdateOne>[1];
+
+  it("applies a title-only update to the node's name (icon untouched)", () => {
+    const tree = buildTree();
+    const next = applyUpdateOne(tree, ev("child", { title: "Renamed" }));
+    const child = treeModel.find(next, "child");
+    expect(child?.name).toBe("Renamed");
+    // Icon is left as it was.
+    expect(child?.icon).toBe("📄");
+  });
+
+  it("applies an icon-only update to the node's icon (name untouched)", () => {
+    const tree = buildTree();
+    const next = applyUpdateOne(tree, ev("root", { icon: "🔥" }));
+    const root = treeModel.find(next, "root");
+    expect(root?.icon).toBe("🔥");
+    expect(root?.name).toBe("Root");
+  });
+
+  it("applies a combined title + icon update", () => {
+    const tree = buildTree();
+    const next = applyUpdateOne(tree, ev("child", { title: "Both", icon: "⭐" }));
+    const child = treeModel.find(next, "child");
+    expect(child?.name).toBe("Both");
+    expect(child?.icon).toBe("⭐");
+  });
+
+  it("returns prev UNCHANGED (same reference) when the id is not loaded", () => {
+    const tree = buildTree();
+    const next = applyUpdateOne(tree, ev("ghost", { title: "Nope" }));
+    expect(next).toBe(tree);
+  });
+
+  it("returns prev UNCHANGED (same reference) for a no-op payload (no title/icon)", () => {
+    // The node exists, but the payload carries neither title nor icon -> nothing
+    // to patch, so the reducer must hand back the same array reference.
+    const tree = buildTree();
+    const next = applyUpdateOne(tree, ev("child", {}));
+    expect(next).toBe(tree);
+  });
+
+  it("treats an explicit null icon/title as a value to apply (undefined check, not truthiness)", () => {
+    // The reducer guards on `!== undefined`, so a clearing null IS applied.
+    const tree = buildTree();
+    const next = applyUpdateOne(tree, ev("child", { title: "", icon: null }));
+    const child = treeModel.find(next, "child");
+    expect(child?.name).toBe("");
+    expect(child?.icon).toBeNull();
+    // And it did change something -> a fresh reference, not prev.
+    expect(next).not.toBe(tree);
+  });
+});

apps/client/src/features/workspace/components/settings/components/ai-provider-settings.spec.tsx

@@ -3,6 +3,9 @@ import {
   resolveCardStatus,
   isEndpointConfigured,
   resolveKeyField,
+  nextReindexPollInterval,
+  isReindexComplete,
+  isReindexButtonLoading,
 } from './ai-provider-settings';
 
 describe('resolveCardStatus', () => {
@@ -71,3 +74,152 @@ describe('resolveKeyField (write-only key payload)', () => {
     expect(resolveKeyField('', false)).toEqual({ set: false });
   });
 });
+
+describe('nextReindexPollInterval', () => {
+  const INTERVAL = 5000;
+  const base = { now: 1_000, intervalMs: INTERVAL };
+
+  it('does not poll when no reindex deadline is set', () => {
+    expect(
+      nextReindexPollInterval({
+        ...base,
+        deadline: null,
+        status: { reindexing: true, indexedPages: 0, totalPages: 478 },
+      }),
+    ).toBe(false);
+  });
+
+  it('keeps polling while the server reports an active run', () => {
+    expect(
+      nextReindexPollInterval({
+        ...base,
+        deadline: 10_000,
+        status: { reindexing: true, indexedPages: 120, totalPages: 478 },
+      }),
+    ).toBe(INTERVAL);
+  });
+
+  it('keeps polling during an active run even if counts momentarily look full', () => {
+    // The run clears its progress record only at the very end, so a transient
+    // indexed==total while reindexing is still true must NOT stop polling.
+    expect(
+      nextReindexPollInterval({
+        ...base,
+        deadline: 10_000,
+        status: { reindexing: true, indexedPages: 478, totalPages: 478 },
+      }),
+    ).toBe(INTERVAL);
+  });
+
+  it('stops once the run is finished AND fully indexed', () => {
+    expect(
+      nextReindexPollInterval({
+        ...base,
+        deadline: 10_000,
+        status: { reindexing: false, indexedPages: 478, totalPages: 478 },
+      }),
+    ).toBe(false);
+  });
+
+  it('keeps polling within the deadline when not yet done and no active flag', () => {
+    // First poll right after enqueue, before the worker publishes progress.
+    expect(
+      nextReindexPollInterval({
+        ...base,
+        deadline: 10_000,
+        status: { reindexing: false, indexedPages: 0, totalPages: 478 },
+      }),
+    ).toBe(INTERVAL);
+  });
+
+  it('cap always wins: stops once past the deadline even if still reindexing', () => {
+    expect(
+      nextReindexPollInterval({
+        deadline: 1_000,
+        now: 2_000, // past the deadline
+        intervalMs: INTERVAL,
+        status: { reindexing: true, indexedPages: 200, totalPages: 478 },
+      }),
+    ).toBe(false);
+  });
+
+  it('stops on an empty workspace (0 of 0) once the run is finished', () => {
+    expect(
+      nextReindexPollInterval({
+        ...base,
+        deadline: 10_000,
+        status: { reindexing: false, indexedPages: 0, totalPages: 0 },
+      }),
+    ).toBe(false);
+  });
+});
+
+describe('isReindexComplete', () => {
+  it('false when no status yet', () => {
+    expect(isReindexComplete(undefined)).toBe(false);
+  });
+
+  it('false while a run is still active (even at indexed==total)', () => {
+    expect(
+      isReindexComplete({ reindexing: true, indexedPages: 478, totalPages: 478 }),
+    ).toBe(false);
+  });
+
+  it('false when finished but not yet fully indexed', () => {
+    expect(
+      isReindexComplete({ reindexing: false, indexedPages: 120, totalPages: 478 }),
+    ).toBe(false);
+  });
+
+  it('true once finished and fully indexed', () => {
+    expect(
+      isReindexComplete({ reindexing: false, indexedPages: 478, totalPages: 478 }),
+    ).toBe(true);
+  });
+});
+
+describe('isReindexButtonLoading', () => {
+  it('loads while the POST mutation is pending', () => {
+    expect(
+      isReindexButtonLoading({
+        mutationPending: true,
+        deadline: null,
+        status: false,
+      }),
+    ).toBe(true);
+  });
+
+  it('does NOT load post-cap: deadline nulled but reindexing left stale-true', () => {
+    // The key case: after the poll cap fires `reindexDeadline` is null while
+    // `settings.reindexing` can be a stale `true` from the last poll. Gating on
+    // the deadline keeps the spinner from sticking forever so the admin can
+    // restart.
+    expect(
+      isReindexButtonLoading({
+        mutationPending: false,
+        deadline: null,
+        status: true,
+      }),
+    ).toBe(false);
+  });
+
+  it('loads during an active run within the poll window', () => {
+    expect(
+      isReindexButtonLoading({
+        mutationPending: false,
+        deadline: 10_000,
+        status: true,
+      }),
+    ).toBe(true);
+  });
+
+  it('does not load once the run finished while still polling', () => {
+    expect(
+      isReindexButtonLoading({
+        mutationPending: false,
+        deadline: 10_000,
+        status: false,
+      }),
+    ).toBe(false);
+  });
+});

apps/client/src/features/workspace/components/settings/components/ai-provider-settings.tsx

@@ -37,6 +37,7 @@ import {
 } from "@/features/workspace/queries/ai-settings-query.ts";
 import {
   AiTestCapability,
+  IAiSettings,
   IAiSettingsUpdate,
   SttApiStyle,
   ChatApiStyle,
@@ -169,6 +170,73 @@ export function resolveKeyField(
   return { set: false };
 }
 
+// Subset of the status payload that drives the reindex poll decisions.
+type ReindexStatus = Pick<
+  IAiSettings,
+  "reindexing" | "indexedPages" | "totalPages"
+>;
+
+/**
+ * Decide the TanStack Query `refetchInterval` while a reindex may be running.
+ * Returns the poll interval (ms) to keep polling, or `false` to stop.
+ *
+ * Polls while the server reports an ACTIVE run (`reindexing === true`) OR we are
+ * still within the deadline window and not yet fully indexed. Stops once the run
+ * has finished AND everything is indexed (server cleared its progress record and
+ * fell back to the DB coverage count), or the deadline cap is hit — the cap
+ * always wins so a stuck/never-clearing progress record can't poll forever.
+ */
+export function nextReindexPollInterval(args: {
+  deadline: number | null;
+  now: number;
+  intervalMs: number;
+  status?: ReindexStatus;
+}): number | false {
+  const { deadline, now, intervalMs, status } = args;
+  if (deadline === null) return false;
+  // Cap always wins.
+  if (now > deadline) return false;
+  // Active run → keep polling even if the momentary counts already look full.
+  if (status?.reindexing) return intervalMs;
+  // Finished and fully indexed (incl. an empty workspace, 0 >= 0) → stop. Reuse
+  // isReindexComplete so the completeness check lives in exactly one place.
+  if (isReindexComplete(status)) return false;
+  // Within the deadline and not yet done → keep polling.
+  return intervalMs;
+}
+
+/**
+ * Whether the reindex poll deadline should be cleared: the server reports no
+ * active run AND the count is complete. The single source of truth for the
+ * "reindex finished" check — `nextReindexPollInterval` reuses it for its stop
+ * condition (sans the cap, which the effect handles via time).
+ */
+export function isReindexComplete(status?: ReindexStatus): boolean {
+  return (
+    !!status && !status.reindexing && status.indexedPages >= status.totalPages
+  );
+}
+
+/**
+ * Whether the reindex button should show its spinner (and stay disabled).
+ *
+ * Spins while the POST is in flight, and for the WHOLE background run while the
+ * server reports `reindexing === true`. The `deadline !== null` gate is the
+ * load-bearing part: once the 120s poll cap fires it nulls `reindexDeadline`
+ * and stops refetching, so `status` (settings?.reindexing) can be a stale
+ * `true` from the last poll. Without the gate the spinner would stick forever
+ * for a run that outlives the cap and block a restart; gating on the active
+ * poll window clears it so the admin can re-trigger.
+ */
+export function isReindexButtonLoading(args: {
+  mutationPending: boolean;
+  deadline: number | null;
+  status?: boolean;
+}): boolean {
+  const { mutationPending, deadline, status } = args;
+  return mutationPending || (deadline !== null && status === true);
+}
+
 // Translate the dot's tooltip label. Kept in one place so all three endpoint
 // cards share identical wording.
 function cardStatusLabel(status: CardStatus, t: (k: string) => string): string {
@@ -215,31 +283,34 @@ export default function AiProviderSettings() {
   // PRE-job counts immediately, so the only way the "Indexed X of Y" counter
   // visibly climbs is to keep polling the settings query while the job runs.
   // `reindexDeadline` is the timestamp until which we poll (set on reindex
-  // success); polling stops early once indexed === total. Bounded so a stuck
-  // job can never poll forever.
-  const REINDEX_POLL_INTERVAL = 3000; // ms between refetches while indexing
+  // success). Polling tracks the server's `reindexing` flag: it keeps going for
+  // the whole active run and stops promptly once the server reports the run is
+  // finished. Bounded by the cap so a stuck/never-clearing progress record can
+  // never poll forever.
+  const REINDEX_POLL_INTERVAL = 5000; // ms between refetches while indexing
   const REINDEX_POLL_CAP_MS = 120000; // ~2 min hard cap
   const [reindexDeadline, setReindexDeadline] = useState<number | null>(null);
 
   // Only admins may read the (masked) AI settings; the server enforces this too.
-  const { data: settings, isLoading } = useAiSettingsQuery(isAdmin, (query) => {
-    if (reindexDeadline === null) return false;
-    // Past the cap → stop polling (cleared via the effect below too).
-    if (Date.now() > reindexDeadline) return false;
-    const data = query.state.data;
-    // Stop once everything is indexed; otherwise keep polling.
-    if (data && data.indexedPages >= data.totalPages) return false;
-    return REINDEX_POLL_INTERVAL;
-  });
+  const { data: settings, isLoading } = useAiSettingsQuery(isAdmin, (query) =>
+    nextReindexPollInterval({
+      deadline: reindexDeadline,
+      now: Date.now(),
+      intervalMs: REINDEX_POLL_INTERVAL,
+      status: query.state.data,
+    }),
+  );
 
-  // Stop polling once the work is done or the cap is reached. Also clears on
+  // Stop polling once the run is finished or the cap is reached. Also clears on
   // unmount because the deadline state goes away with the component.
   useEffect(() => {
     if (reindexDeadline === null) return;
-    // "Done" matches the refetchInterval stop condition (indexed >= total),
-    // including an empty workspace (0 >= 0), so the deadline clears promptly
-    // instead of waiting out the cap.
-    if (settings && settings.indexedPages >= settings.totalPages) {
+    // "Done" matches the refetchInterval stop condition: the server reports no
+    // active run AND the count is complete (indexed >= total, incl. an empty
+    // workspace 0 >= 0), so the deadline clears promptly instead of waiting out
+    // the cap. While `reindexing` is still true we keep the deadline so polling
+    // continues for the whole run.
+    if (isReindexComplete(settings)) {
       setReindexDeadline(null);
       return;
     }
@@ -1031,7 +1102,17 @@ export default function AiProviderSettings() {
             <Button
               variant="subtle"
               size="compact-sm"
-              loading={reindexMutation.isPending}
+              // Spin for the WHOLE run: the POST resolves immediately, but the
+              // background job keeps running, so also stay loading while the
+              // server reports `reindexing` (this also blocks a redundant
+              // re-trigger mid-run; the server de-dupes regardless). The
+              // deadline gate (and why it matters post-cap) lives in
+              // `isReindexButtonLoading`, which is unit-tested.
+              loading={isReindexButtonLoading({
+                mutationPending: reindexMutation.isPending,
+                deadline: reindexDeadline,
+                status: settings?.reindexing,
+              })}
               onClick={() =>
                 reindexMutation.mutate(undefined, {
                   // Begin bounded polling so the counter climbs as the async

apps/client/src/features/workspace/queries/ai-settings-query.ts

@@ -23,8 +23,12 @@ export function useAiSettingsQuery(
   enabled: boolean = true,
   // While reindexing runs as an async background job, the counter only climbs
   // if the client keeps refetching. The component passes a refetchInterval
-  // function that polls until indexed === total or a bounded deadline, then
-  // returns false to stop. See AiProviderSettings.
+  // function (`nextReindexPollInterval`) that keeps polling while the server
+  // reports an active run (reindexing === true) OR we are still within the
+  // bounded deadline and not yet fully indexed; it returns false to stop only
+  // once the run has finished AND indexed >= total, or the deadline cap is hit
+  // (the cap always wins). Note: a transient indexed === total during an active
+  // run does NOT stop polling. See AiProviderSettings.
   refetchInterval?:
     | number
     | false

apps/client/src/features/workspace/services/ai-settings-service.ts

@@ -48,6 +48,9 @@ export interface IAiSettings {
   // RAG indexing coverage (pages indexed for semantic search).
   indexedPages: number;
   totalPages: number;
+  // True while a full workspace reindex is actively running; the counts above
+  // then reflect the live run progress (done climbs 0 -> total).
+  reindexing?: boolean;
 }
 
 // Update payload. Key semantics (same for `apiKey` and `embeddingApiKey`):

apps/server/src/collaboration/collaboration.util.ts

@@ -36,7 +36,6 @@ import {
   Mention,
   Subpages,
   Highlight,
-  Spoiler,
   Indent,
   UniqueID,
   Columns,
@@ -83,7 +82,6 @@ export const tiptapExtensions = [
   Superscript,
   SubScript,
   Highlight,
-  Spoiler,
   Typography,
   TrailingNode,
   TextStyle,

apps/server/src/collaboration/extensions/persistence-store.spec.ts

@@ -205,31 +205,203 @@ describe('PersistenceExtension.onStoreDocument — Approach-A boundary snapshot'
     expect(historyQueue.add).toHaveBeenCalledTimes(1);
   });
 
-  // #206 persist-6 — RED (it.failing): a momentarily-empty live Y.Doc must not
-  // overwrite non-empty persisted content. `onStoreDocument` empty-guards the
-  // LOAD path but not the STORE path, so today an empty doc (a client/agent
-  // glitch, a bad merge, an emptying transclusion) is written straight over the
-  // page and the content is wiped silently. A store-side empty-guard is a real
-  // behaviour change (a deliberate "select-all + delete" is also empty), so it
-  // is left UNFIXED pending a product decision; this documents the data-loss
-  // path and flips to a normal passing test the moment the guard lands.
-  it.failing(
-    'does NOT overwrite non-empty content with a momentarily-empty live doc (persist-6)',
-    async () => {
-      const emptyDoc = { type: 'doc', content: [{ type: 'paragraph' }] };
-      const document = ydocFor(emptyDoc);
-      pageRepo.findById.mockResolvedValue({
-        ...persistedHumanPage('IGNORED'),
-        content: doc('IMPORTANT RICH CONTENT'),
-      });
+  // #206 persist-6 / #248 — a momentarily-empty live Y.Doc must not overwrite
+  // non-empty persisted content. The store-side empty-guard blocks an empty doc
+  // (a client/agent glitch, a bad merge, an emptying transclusion) from wiping
+  // the page silently when NO intentional-clear signal is present.
+  it('does NOT overwrite non-empty content with a momentarily-empty live doc (persist-6)', async () => {
+    const emptyDoc = { type: 'doc', content: [{ type: 'paragraph' }] };
+    const document = ydocFor(emptyDoc);
+    pageRepo.findById.mockResolvedValue({
+      ...persistedHumanPage('IGNORED'),
+      content: doc('IMPORTANT RICH CONTENT'),
+    });
 
-      await ext.onStoreDocument(buildData(document, 'user') as any);
+    await ext.onStoreDocument(buildData(document, 'user') as any);
 
-      // Desired contract: the empty incoming doc is rejected and the rich page
-      // survives. Today updatePage is called with the empty content (data loss).
-      expect(pageRepo.updatePage).not.toHaveBeenCalled();
-    },
-  );
+    // The empty incoming doc is rejected and the rich page survives.
+    expect(pageRepo.updatePage).not.toHaveBeenCalled();
+  });
+
+  // #248 — an empty-over-empty store is allowed (nothing to lose); the guard
+  // only protects non-empty persisted content.
+  it('allows an empty store over already-empty content (#248)', async () => {
+    const liveEmptyDoc = { type: 'doc', content: [{ type: 'paragraph' }] };
+    const document = ydocFor(liveEmptyDoc);
+    // Stored content is empty per isEmptyParagraphDoc (paragraph with content:[])
+    // but NOT deep-equal to the normalized live doc, so the unchanged
+    // short-circuit is skipped and the empty-guard is genuinely reached.
+    pageRepo.findById.mockResolvedValue({
+      ...persistedHumanPage('IGNORED'),
+      content: { type: 'doc', content: [{ type: 'paragraph', content: [] }] },
+    });
+
+    await ext.onStoreDocument(buildData(document, 'user') as any);
+
+    expect(pageRepo.updatePage).toHaveBeenCalledTimes(1);
+  });
+
+  // #251 — REAL-PATH regression test. The intentional-clear signal is set via
+  // the actual transport seam (ext.onStateless with the exact stateless payload
+  // the client's IntentionalClear extension sends), NOT a hand-injected
+  // context.intentionalClear poke. We then run the debounced store with an empty
+  // live doc over non-empty persisted content and assert the empty write goes
+  // through — i.e. the clear persists.
+  it('persists an intentional clear signalled via the real stateless transport (#251)', async () => {
+    const documentName = `page.${PAGE_ID}`;
+    const emptyDoc = { type: 'doc', content: [{ type: 'paragraph' }] };
+    const document = ydocFor(emptyDoc);
+    pageRepo.findById.mockResolvedValue({
+      ...persistedHumanPage('IGNORED'),
+      content: doc('IMPORTANT RICH CONTENT'),
+    });
+
+    // The client signalled a deliberate clear over the live connection.
+    await ext.onStateless({
+      connection: { readOnly: false } as any,
+      documentName,
+      document: document as any,
+      payload: JSON.stringify({ type: 'intentional-clear' }),
+    } as any);
+
+    await ext.onStoreDocument(buildData(document, 'user') as any);
+
+    // The empty doc was written (the clear persisted). The persisted content is
+    // the Y.Doc round-trip of the empty doc (attrs normalized), so compare
+    // against fromYdoc rather than the raw literal.
+    expect(pageRepo.updatePage).toHaveBeenCalledTimes(1);
+    const expectedEmpty = TiptapTransformer.fromYdoc(document, 'default');
+    expect(pageRepo.updatePage.mock.calls[0][0].content).toEqual(expectedEmpty);
+  });
+
+  // #251 — retry correctness: a transient DB failure on the FIRST attempt must
+  // not silently drop the clear. The intentional-clear flag is consumed ONCE
+  // before the retry loop, so when attempt 1's updatePage throws (tx rolls back,
+  // but the in-memory flag delete cannot roll back) the retry on attempt 2 still
+  // sees the clear as allowed and writes the empty doc. On the pre-fix code
+  // (consumeIntentionalClear called INSIDE the loop) attempt 1 consumed the flag,
+  // attempt 2 re-read it as absent and the empty-guard BLOCKED the write — so
+  // updatePage would be called once and the clear would be lost. This test fails
+  // on that ordering and passes after the hoist.
+  it('persists an intentional clear even when the first store attempt fails transiently (#251)', async () => {
+    const documentName = `page.${PAGE_ID}`;
+    const emptyDoc = { type: 'doc', content: [{ type: 'paragraph' }] };
+    const document = ydocFor(emptyDoc);
+    // The page stays non-empty in the DB across both attempts (the rolled-back
+    // first attempt never changed it), exactly the failure scenario the WARNING
+    // describes.
+    pageRepo.findById.mockResolvedValue({
+      ...persistedHumanPage('IGNORED'),
+      content: doc('IMPORTANT RICH CONTENT'),
+    });
+
+    let attempts = 0;
+    pageRepo.updatePage.mockImplementation(async () => {
+      attempts += 1;
+      if (attempts === 1) throw new Error('deadlock detected'); // transient
+      callOrder.push('updatePage');
+    });
+
+    // The client signalled a deliberate clear over the live connection.
+    await ext.onStateless({
+      connection: { readOnly: false } as any,
+      documentName,
+      document: document as any,
+      payload: JSON.stringify({ type: 'intentional-clear' }),
+    } as any);
+
+    await ext.onStoreDocument(buildData(document, 'user') as any);
+
+    // First attempt failed and rolled back; the retry still honoured the clear
+    // and wrote the empty doc (the clear survived the retry).
+    expect(pageRepo.updatePage).toHaveBeenCalledTimes(2);
+    const expectedEmpty = TiptapTransformer.fromYdoc(document, 'default');
+    expect(pageRepo.updatePage.mock.calls[1][0].content).toEqual(expectedEmpty);
+  });
+
+  // #251 — the signal is single-use: it is consumed by the first empty store,
+  // so a SECOND accidental empty (no fresh signal) is still blocked.
+  it('consumes the intentional-clear signal once; a later empty is blocked (#251)', async () => {
+    const documentName = `page.${PAGE_ID}`;
+    const emptyDoc = { type: 'doc', content: [{ type: 'paragraph' }] };
+    pageRepo.findById.mockResolvedValue({
+      ...persistedHumanPage('IGNORED'),
+      content: doc('IMPORTANT RICH CONTENT'),
+    });
+
+    await ext.onStateless({
+      connection: { readOnly: false } as any,
+      documentName,
+      document: ydocFor(emptyDoc) as any,
+      payload: JSON.stringify({ type: 'intentional-clear' }),
+    } as any);
+
+    // First empty store consumes the signal and writes.
+    await ext.onStoreDocument(buildData(ydocFor(emptyDoc), 'user') as any);
+    expect(pageRepo.updatePage).toHaveBeenCalledTimes(1);
+
+    // Re-arm findById to non-empty (as if content came back) and fire another
+    // empty store WITHOUT a new signal — the guard must block it.
+    pageRepo.updatePage.mockClear();
+    pageRepo.findById.mockResolvedValue({
+      ...persistedHumanPage('IGNORED'),
+      content: doc('IMPORTANT RICH CONTENT'),
+    });
+    await ext.onStoreDocument(buildData(ydocFor(emptyDoc), 'user') as any);
+    expect(pageRepo.updatePage).not.toHaveBeenCalled();
+  });
+
+  // #251 — a read-only connection cannot arm the clear, so its empty store is
+  // still blocked (defends the guard against a read-only spoof).
+  it('ignores an intentional-clear signal from a read-only connection (#251)', async () => {
+    const documentName = `page.${PAGE_ID}`;
+    const emptyDoc = { type: 'doc', content: [{ type: 'paragraph' }] };
+    const document = ydocFor(emptyDoc);
+    pageRepo.findById.mockResolvedValue({
+      ...persistedHumanPage('IGNORED'),
+      content: doc('IMPORTANT RICH CONTENT'),
+    });
+
+    await ext.onStateless({
+      connection: { readOnly: true } as any,
+      documentName,
+      document: document as any,
+      payload: JSON.stringify({ type: 'intentional-clear' }),
+    } as any);
+
+    await ext.onStoreDocument(buildData(document, 'user') as any);
+
+    expect(pageRepo.updatePage).not.toHaveBeenCalled();
+  });
+
+  // #251 — a non-empty store between the signal and the empty store drops the
+  // pending flag ("cleared then retyped" can't leave a usable signal behind).
+  it('drops a pending clear when a non-empty store intervenes (#251)', async () => {
+    const documentName = `page.${PAGE_ID}`;
+    const emptyDoc = { type: 'doc', content: [{ type: 'paragraph' }] };
+
+    await ext.onStateless({
+      connection: { readOnly: false } as any,
+      documentName,
+      document: ydocFor(emptyDoc) as any,
+      payload: JSON.stringify({ type: 'intentional-clear' }),
+    } as any);
+
+    // A non-empty store lands first → consumes/drops the stale flag.
+    pageRepo.findById.mockResolvedValue(persistedHumanPage('NEW HUMAN TEXT'));
+    await ext.onStoreDocument(
+      buildData(ydocFor(doc('NEW HUMAN TEXT')), 'user') as any,
+    );
+    pageRepo.updatePage.mockClear();
+
+    // Now an empty store with no fresh signal must be blocked.
+    pageRepo.findById.mockResolvedValue({
+      ...persistedHumanPage('IGNORED'),
+      content: doc('IMPORTANT RICH CONTENT'),
+    });
+    await ext.onStoreDocument(buildData(ydocFor(emptyDoc), 'user') as any);
+    expect(pageRepo.updatePage).not.toHaveBeenCalled();
+  });
 
   // persist-1 — when every attempt fails the hook must NOT report a phantom
   // success: no "page.updated" badge broadcast and no history snapshot for

apps/server/src/collaboration/extensions/persistence.extension.ts

@@ -3,6 +3,7 @@ import {
   Extension,
   onChangePayload,
   onLoadDocumentPayload,
+  onStatelessPayload,
   onStoreDocumentPayload,
 } from '@hocuspocus/server';
 import * as Y from 'yjs';
@@ -41,6 +42,35 @@ import {
 } from '../constants';
 import { TransclusionService } from '../../core/page/transclusion/transclusion.service';
 
+/**
+ * #251 — wire format of the client→server stateless message that signals a
+ * deliberate page clear. The client (IntentionalClear editor extension) sends
+ * `{ type: INTENTIONAL_CLEAR_MESSAGE_TYPE }`; the document is taken from the
+ * connection, not the payload, so the signal cannot be aimed at another page.
+ */
+export const INTENTIONAL_CLEAR_MESSAGE_TYPE = 'intentional-clear';
+
+/**
+ * #251 — how long an intentional-clear signal stays "pending" before it is
+ * ignored. The signal is set on the clearing keystroke but consumed by the
+ * DEBOUNCED onStoreDocument, so the TTL must comfortably exceed the collab
+ * store debounce window (hocuspocus is configured with maxDebounce = 45s in
+ * collaboration.gateway.ts). 60s leaves a margin while keeping the window for a
+ * stale flag small; on top of the TTL, any non-empty store immediately drops a
+ * pending flag (see onStoreDocument), so a "cleared then retyped" sequence can
+ * never leave a usable flag behind.
+ *
+ * Known fail-safe limitation: the flag lives only in this node's process memory.
+ * If document ownership transfers to another node, or this node crashes/restarts,
+ * between the stateless signal (set on node A) and the debounced store, the
+ * in-memory flag is lost and the clear is silently NOT applied — the store-side
+ * empty-guard then reloads the document non-empty from the DB. This is
+ * deliberately fail-safe (a lost flag preserves content rather than destroying
+ * it), but it is a documented limitation, not a guarantee that every deliberate
+ * clear survives a node handoff.
+ */
+export const INTENTIONAL_CLEAR_TTL_MS = 60_000;
+
 /**
  * Resolve the provenance source for a coalesced snapshot.
  *
@@ -96,6 +126,13 @@ export class PersistenceExtension implements Extension {
   // coalescing window" per document and OR it across all edits in the window,
   // so the snapshot is marked 'agent' regardless of who wrote last.
   private agentTouched: Map<string, boolean> = new Map();
+  // #251 — per-document "intentional clear pending" flags. Keyed by
+  // documentName, value = expiry timestamp (ms). Set by onStateless when the
+  // client reports a deliberate clear; consumed once by the next
+  // onStoreDocument empty-guard branch. This is the per-EDIT channel the
+  // per-connection context cannot provide (a clear is an edit event, but the
+  // store is debounced and connection context is fixed at authentication).
+  private intentionalClear: Map<string, number> = new Map();
 
   constructor(
     private readonly pageRepo: PageRepo,
@@ -180,6 +217,19 @@ export class PersistenceExtension implements Extension {
       this.consumeAgentTouched(documentName),
       context?.actor,
     );
+    // #251 — consume the intentional-clear flag ONCE, BEFORE the retry loop
+    // (like consumeContributors / consumeAgentTouched above). consumeIntentional-
+    // Clear ALWAYS deletes the in-memory Map entry, but a tx rollback cannot
+    // un-delete it. Calling it INSIDE the loop meant: a clear armed for attempt 1
+    // was consumed there, attempt 1's updatePage threw a transient error and
+    // rolled back, then attempt 2 re-read non-empty content and saw the flag
+    // already gone — silently downgrading the retry into a BLOCKED write, so the
+    // user's deliberate clear was dropped. Hoisting makes the decision stable
+    // across every attempt. This single call also preserves the "a non-empty
+    // store drops a pending flag" semantics (the cleared-then-retyped case):
+    // every store consumes the flag here regardless of incoming emptiness, so a
+    // subsequent non-empty store can never leave a usable flag behind.
+    const allowIntentionalClear = this.consumeIntentionalClear(documentName);
 
     // Persist with a small bounded retry. The in-memory Y.Doc is the ONLY copy
     // of the latest edit until this hook returns: hocuspocus destroys/unloads the
@@ -210,6 +260,46 @@ export class PersistenceExtension implements Extension {
             return;
           }
 
+          // #206 persist-6 / #248 — store-side empty-guard. A momentarily-empty
+          // live Y.Doc (a client/agent glitch, a bad merge, a transclusion that
+          // emptied) must NOT overwrite non-empty persisted content. The LOAD
+          // path already guards emptiness (onLoadDocument only hydrates from db
+          // when the live doc isEmpty); the STORE path did not, so an empty
+          // serialization was written straight over the page, wiping it
+          // silently.
+          //
+          // #251 — the ONE legitimate empty-over-non-empty write is a user who
+          // deliberately clears the page. That intent arrives out-of-band as a
+          // stateless message, NOT from the doc content, which is why it cannot
+          // be spoofed for non-clear writes: the flag is only ever read on this
+          // empty-incoming branch, so the worst a forged signal can do is clear
+          // a page the connection may already edit. The flag was consumed ONCE
+          // before the retry loop (`allowIntentionalClear`) so the decision is
+          // stable across retries; a non-empty store still drops any pending
+          // flag via that same hoisted consume (a "cleared then retyped"
+          // sequence can't leave a usable one behind).
+          const incomingEmpty = isEmptyParagraphDoc(tiptapJson as any);
+          if (
+            incomingEmpty &&
+            page.content &&
+            !isEmptyParagraphDoc(page.content as any)
+          ) {
+            if (allowIntentionalClear) {
+              this.logger.debug(
+                `Intentional clear for ${pageId}: persisting empty doc over ` +
+                  `non-empty content (user-signalled)`,
+              );
+              // fall through — the empty write is allowed exactly once.
+            } else {
+              this.logger.warn(
+                `Skipping store for ${pageId}: empty live doc would overwrite ` +
+                  `non-empty persisted content`,
+              );
+              page = null;
+              return;
+            }
+          }
+
           let contributorIds = undefined;
           try {
             const existingContributors = page.contributorIds || [];
@@ -345,6 +435,37 @@ export class PersistenceExtension implements Extension {
     }
   }
 
+  /**
+   * #251 — receive the client's deliberate-clear signal. Records a short-lived,
+   * single-use pending flag for the originating document so the next
+   * onStoreDocument may let one empty-over-non-empty write through the guard.
+   *
+   * Hardening: read-only connections cannot arm the flag, and the document is
+   * taken from the connection (`data.documentName`), never the payload, so a
+   * client cannot target a page it isn't editing. The flag only ever RELAXES
+   * the guard for an empty write (a clear); it can never force or alter a
+   * non-empty write, so it is not a guard bypass for normal content.
+   */
+  async onStateless(data: onStatelessPayload) {
+    const { connection, documentName, payload } = data;
+
+    if (connection?.readOnly) return;
+
+    let message: { type?: string } | undefined;
+    try {
+      message = JSON.parse(payload);
+    } catch {
+      return; // unrelated / malformed stateless message
+    }
+
+    if (message?.type !== INTENTIONAL_CLEAR_MESSAGE_TYPE) return;
+
+    this.intentionalClear.set(
+      documentName,
+      Date.now() + INTENTIONAL_CLEAR_TTL_MS,
+    );
+  }
+
   async onChange(data: onChangePayload) {
     const documentName = data.documentName;
     const userId = data.context?.user?.id;
@@ -368,6 +489,7 @@ export class PersistenceExtension implements Extension {
     const documentName = data.documentName;
     this.contributors.delete(documentName);
     this.agentTouched.delete(documentName);
+    this.intentionalClear.delete(documentName);
   }
 
   private consumeContributors(documentName: string): string[] {
@@ -385,6 +507,18 @@ export class PersistenceExtension implements Extension {
     return touched;
   }
 
+  /**
+   * #251 — read and clear the intentional-clear flag for this document. Returns
+   * true only if a flag was pending AND still within its TTL. Always deletes the
+   * entry so the signal is strictly single-use (one clear → one allowed empty
+   * write); an expired flag is treated as absent (guard still blocks).
+   */
+  private consumeIntentionalClear(documentName: string): boolean {
+    const expiry = this.intentionalClear.get(documentName);
+    this.intentionalClear.delete(documentName);
+    return expiry !== undefined && Date.now() < expiry;
+  }
+
   private async enqueuePageHistory(
     page: Page,
     lastUpdatedSource: string,

apps/server/src/collaboration/yjs.util.spec.ts

@@ -0,0 +1,278 @@
+import * as Y from 'yjs';
+import { getSchema } from '@tiptap/core';
+import {
+  initProseMirrorDoc,
+  absolutePositionToRelativePosition,
+  prosemirrorJSONToYDoc,
+} from '@tiptap/y-tiptap';
+import { tiptapExtensions } from './collaboration.util';
+import {
+  setYjsMark,
+  removeYjsMarkByAttribute,
+  updateYjsMarkAttribute,
+  type YjsSelection,
+} from './yjs.util';
+
+/**
+ * Unit tests for the server-side Yjs mark helpers used by the collaboration
+ * handler to set/resolve/delete comment marks directly on the shared Y.Doc
+ * (collaboration.handler.ts: setCommentMark / resolveCommentMark).
+ *
+ * The fragment shape mirrors production exactly: a `default` XmlFragment whose
+ * children are block XmlElements (paragraph) holding XmlText runs. For setYjsMark
+ * the selection is a pair of Yjs RelativePosition JSONs (what the client sends);
+ * we synthesize them from known ProseMirror absolute positions via
+ * absolutePositionToRelativePosition so the marked range is deterministic.
+ */
+
+const schema = getSchema(tiptapExtensions);
+
+// Build a real Y.Doc from ProseMirror JSON (same path the collab handler uses
+// via TiptapTransformer) and return the doc + its `default` fragment.
+function buildFromPm(pmJson: unknown) {
+  const ydoc = prosemirrorJSONToYDoc(
+    schema,
+    pmJson as never,
+    'default',
+  ) as unknown as Y.Doc;
+  const fragment = ydoc.getXmlFragment('default');
+  return { ydoc, fragment };
+}
+
+// Make a YjsSelection (anchor/head RelativePosition JSON) for two ProseMirror
+// absolute positions in `fragment`.
+function selectionFor(
+  fragment: Y.XmlFragment,
+  anchorPos: number,
+  headPos: number,
+): YjsSelection {
+  const { mapping } = initProseMirrorDoc(fragment, schema);
+  const anchor = absolutePositionToRelativePosition(
+    anchorPos,
+    fragment as never,
+    mapping,
+  );
+  const head = absolutePositionToRelativePosition(
+    headPos,
+    fragment as never,
+    mapping,
+  );
+  return {
+    anchor: Y.relativePositionToJSON(anchor),
+    head: Y.relativePositionToJSON(head),
+  };
+}
+
+// The XmlText run of the i-th top-level paragraph.
+function paragraphText(fragment: Y.XmlFragment, index = 0): Y.XmlText {
+  const para = fragment.get(index) as Y.XmlElement;
+  return para.get(0) as Y.XmlText;
+}
+
+// --- raw fragment builder for the remove/update tests (no schema needed) ---
+//
+// removeYjsMarkByAttribute / updateYjsMarkAttribute only read item.toDelta() and
+// call item.format(); they never touch the ProseMirror schema. Build the runs
+// directly so we control which segment carries which comment attrs.
+function buildWithComments(
+  segments: Array<{
+    text: string;
+    comment?: { commentId: string; resolved: boolean };
+  }>,
+): { fragment: Y.XmlFragment; text: Y.XmlText } {
+  const ydoc = new Y.Doc();
+  const fragment = ydoc.getXmlFragment('default');
+  const para = new Y.XmlElement('paragraph');
+  fragment.insert(0, [para]);
+  const text = new Y.XmlText();
+  para.insert(0, [text]);
+  let offset = 0;
+  for (const seg of segments) {
+    text.insert(offset, seg.text);
+    if (seg.comment) {
+      text.format(offset, seg.text.length, { comment: seg.comment });
+    }
+    offset += seg.text.length;
+  }
+  return { fragment, text };
+}
+
+describe('setYjsMark', () => {
+  it('applies the mark over exactly the selected sub-range (PM pos 1..6 = "Hello")', () => {
+    const { ydoc, fragment } = buildFromPm({
+      type: 'doc',
+      content: [
+        { type: 'paragraph', content: [{ type: 'text', text: 'Hello world' }] },
+      ],
+    });
+    // PM pos 1 = start of the paragraph text; pos 6 = just after "Hello".
+    const sel = selectionFor(fragment, 1, 6);
+
+    setYjsMark(ydoc as never, fragment, sel, 'comment', {
+      commentId: 'c1',
+      resolved: false,
+    });
+
+    // The run splits: "Hello" carries the comment mark, " world" stays clean.
+    expect(paragraphText(fragment).toDelta()).toEqual([
+      {
+        insert: 'Hello',
+        attributes: { comment: { commentId: 'c1', resolved: false } },
+      },
+      { insert: ' world' },
+    ]);
+  });
+
+  it('normalizes a reversed selection (head before anchor) to the same range', () => {
+    const { ydoc, fragment } = buildFromPm({
+      type: 'doc',
+      content: [
+        { type: 'paragraph', content: [{ type: 'text', text: 'Hello world' }] },
+      ],
+    });
+    // anchor=6, head=1 — reversed; setYjsMark takes min/max so it marks "Hello".
+    const sel = selectionFor(fragment, 6, 1);
+
+    setYjsMark(ydoc as never, fragment, sel, 'comment', {
+      commentId: 'c2',
+      resolved: false,
+    });
+
+    expect(paragraphText(fragment).toDelta()).toEqual([
+      {
+        insert: 'Hello',
+        attributes: { comment: { commentId: 'c2', resolved: false } },
+      },
+      { insert: ' world' },
+    ]);
+  });
+
+  it('marks across two paragraphs (range spans an element boundary)', () => {
+    const { ydoc, fragment } = buildFromPm({
+      type: 'doc',
+      content: [
+        { type: 'paragraph', content: [{ type: 'text', text: 'aaa' }] },
+        { type: 'paragraph', content: [{ type: 'text', text: 'bbb' }] },
+      ],
+    });
+    // PM positions: "aaa" = 1..4; the </p><p> boundary consumes pos 4 and 5, so
+    // "bbb" starts at pos 6 (chars at 6,7,8). Select pos 2 (inside "aaa") to pos
+    // 8 (after the second "b").
+    const sel = selectionFor(fragment, 2, 8);
+
+    setYjsMark(ydoc as never, fragment, sel, 'comment', {
+      commentId: 'c3',
+      resolved: false,
+    });
+
+    // First paragraph: "a" clean, "aa" marked.
+    expect(paragraphText(fragment, 0).toDelta()).toEqual([
+      { insert: 'a' },
+      {
+        insert: 'aa',
+        attributes: { comment: { commentId: 'c3', resolved: false } },
+      },
+    ]);
+    // Second paragraph: "bb" marked, "b" clean.
+    expect(paragraphText(fragment, 1).toDelta()).toEqual([
+      {
+        insert: 'bb',
+        attributes: { comment: { commentId: 'c3', resolved: false } },
+      },
+      { insert: 'b' },
+    ]);
+  });
+});
+
+describe('removeYjsMarkByAttribute', () => {
+  it('removes only the run whose attribute value matches, leaving others', () => {
+    const { fragment, text } = buildWithComments([
+      { text: 'AAA', comment: { commentId: 'c1', resolved: false } },
+      { text: 'BBB', comment: { commentId: 'c2', resolved: false } },
+    ]);
+
+    removeYjsMarkByAttribute(fragment, 'comment', 'commentId', 'c1');
+
+    // c1's run loses the mark; c2's run is untouched.
+    expect(text.toDelta()).toEqual([
+      { insert: 'AAA' },
+      {
+        insert: 'BBB',
+        attributes: { comment: { commentId: 'c2', resolved: false } },
+      },
+    ]);
+  });
+
+  it('does nothing when no run carries the requested value (no-match branch)', () => {
+    const { fragment, text } = buildWithComments([
+      { text: 'AAA', comment: { commentId: 'c1', resolved: false } },
+    ]);
+    const before = text.toDelta();
+
+    removeYjsMarkByAttribute(fragment, 'comment', 'commentId', 'does-not-exist');
+
+    expect(text.toDelta()).toEqual(before);
+  });
+
+  it('leaves a different mark type alone', () => {
+    // A run carrying only `bold` must survive a comment removal pass.
+    const ydoc = new Y.Doc();
+    const fragment = ydoc.getXmlFragment('default');
+    const para = new Y.XmlElement('paragraph');
+    fragment.insert(0, [para]);
+    const text = new Y.XmlText();
+    para.insert(0, [text]);
+    text.insert(0, 'XYZ');
+    text.format(0, 3, { bold: true });
+
+    removeYjsMarkByAttribute(fragment, 'comment', 'commentId', 'c1');
+
+    expect(text.toDelta()).toEqual([
+      { insert: 'XYZ', attributes: { bold: true } },
+    ]);
+  });
+});
+
+describe('updateYjsMarkAttribute', () => {
+  it('merges new attributes into the matching run, preserving the rest', () => {
+    const { fragment, text } = buildWithComments([
+      { text: 'AAA', comment: { commentId: 'c1', resolved: false } },
+      { text: 'BBB', comment: { commentId: 'c2', resolved: false } },
+    ]);
+
+    updateYjsMarkAttribute(
+      fragment,
+      'comment',
+      { name: 'commentId', value: 'c1' },
+      { resolved: true },
+    );
+
+    // c1's run flips resolved=true (commentId preserved via merge); c2 untouched.
+    expect(text.toDelta()).toEqual([
+      {
+        insert: 'AAA',
+        attributes: { comment: { commentId: 'c1', resolved: true } },
+      },
+      {
+        insert: 'BBB',
+        attributes: { comment: { commentId: 'c2', resolved: false } },
+      },
+    ]);
+  });
+
+  it('does nothing when no run matches (no-match branch)', () => {
+    const { fragment, text } = buildWithComments([
+      { text: 'AAA', comment: { commentId: 'c1', resolved: false } },
+    ]);
+    const before = text.toDelta();
+
+    updateYjsMarkAttribute(
+      fragment,
+      'comment',
+      { name: 'commentId', value: 'nope' },
+      { resolved: true },
+    );
+
+    expect(text.toDelta()).toEqual(before);
+  });
+});

apps/server/src/core/ai-chat/embedding/embedding-indexer.service.spec.ts

@@ -3,6 +3,8 @@ import { PageRepo } from '@docmost/db/repos/page/page.repo';
 import { PageEmbeddingRepo } from '@docmost/db/repos/ai-chat/page-embedding.repo';
 import { KyselyDB } from '@docmost/db/types/kysely.types';
 import { AiService } from '../../../integrations/ai/ai.service';
+import { EmbeddingReindexProgressService } from '../../../integrations/ai/embedding-reindex-progress.service';
+import { AiEmbeddingNotConfiguredException } from '../../../integrations/ai/ai-embedding-not-configured.exception';
 
 /**
  * Unit tests for EmbeddingIndexerService.reindexWorkspace's batch control flow.
@@ -12,7 +14,8 @@ import { AiService } from '../../../integrations/ai/ai.service';
  * reindexWorkspace actually touches:
  *   - aiService.getEmbeddingModel -> a model string so the up-front configured
  *     check passes,
- *   - pageRepo.getIdsByWorkspace -> three page ids,
+ *   - pageRepo.getEmbeddablePageIds -> three page ids (the embeddable set the
+ *     reindex iterates),
  *   - service.reindexPage -> spied per test to drive the per-page outcome.
  *
  * The point under test is the catch block: a FATAL provider error (auth/billing)
@@ -24,21 +27,30 @@ describe('EmbeddingIndexerService.reindexWorkspace fail-fast', () => {
 
   function makeService() {
     const pageRepo = {
-      getIdsByWorkspace: jest.fn().mockResolvedValue(['p1', 'p2', 'p3']),
+      getEmbeddablePageIds: jest.fn().mockResolvedValue(['p1', 'p2', 'p3']),
     };
     const pageEmbeddingRepo = {};
     const aiService = {
       getEmbeddingModel: jest.fn().mockResolvedValue('some-model'),
     };
+    // Progress is a best-effort cosmetic store; mock its async methods so the
+    // batch control flow can be tested without Redis.
+    const reindexProgress = {
+      start: jest.fn().mockResolvedValue(undefined),
+      increment: jest.fn().mockResolvedValue(undefined),
+      clear: jest.fn().mockResolvedValue(undefined),
+      get: jest.fn().mockResolvedValue(null),
+    };
     const db = {};
 
     const service = new EmbeddingIndexerService(
       pageRepo as unknown as PageRepo,
       pageEmbeddingRepo as unknown as PageEmbeddingRepo,
       aiService as unknown as AiService,
+      reindexProgress as unknown as EmbeddingReindexProgressService,
       db as unknown as KyselyDB,
     );
-    return { service, pageRepo, aiService };
+    return { service, pageRepo, aiService, reindexProgress };
   }
 
   it('aborts after the first page on a FATAL (401) provider error', async () => {
@@ -78,3 +90,100 @@ describe('EmbeddingIndexerService.reindexWorkspace fail-fast', () => {
     expect(reindexPage).toHaveBeenCalledTimes(3);
   });
 });
+
+/**
+ * Live reindex-progress reporting: reindexWorkspace must publish a per-workspace
+ * progress record (total at start, done incremented per processed page) and ALWAYS
+ * clear it in a finally — including on a fatal abort and an unconfigured early
+ * return — so the settings status can show the counter climb without ever getting
+ * stuck in a "reindexing" state.
+ */
+describe('EmbeddingIndexerService.reindexWorkspace progress', () => {
+  const WORKSPACE_ID = 'ws-1';
+
+  function makeService(pageIds: string[] = ['p1', 'p2', 'p3']) {
+    const pageRepo = {
+      getEmbeddablePageIds: jest.fn().mockResolvedValue(pageIds),
+    };
+    const pageEmbeddingRepo = {};
+    const aiService = {
+      getEmbeddingModel: jest.fn().mockResolvedValue('some-model'),
+    };
+    const reindexProgress = {
+      start: jest.fn().mockResolvedValue(undefined),
+      increment: jest.fn().mockResolvedValue(undefined),
+      clear: jest.fn().mockResolvedValue(undefined),
+      get: jest.fn().mockResolvedValue(null),
+    };
+    const db = {};
+    const service = new EmbeddingIndexerService(
+      pageRepo as unknown as PageRepo,
+      pageEmbeddingRepo as unknown as PageEmbeddingRepo,
+      aiService as unknown as AiService,
+      reindexProgress as unknown as EmbeddingReindexProgressService,
+      db as unknown as KyselyDB,
+    );
+    return { service, pageRepo, aiService, reindexProgress };
+  }
+
+  it('sets total at start, increments done per page, and clears in finally', async () => {
+    const { service, reindexProgress } = makeService(['p1', 'p2', 'p3']);
+    jest.spyOn(service, 'reindexPage').mockResolvedValue(undefined);
+
+    await service.reindexWorkspace(WORKSPACE_ID);
+
+    expect(reindexProgress.start).toHaveBeenCalledWith(WORKSPACE_ID, 3);
+    // One increment per processed page.
+    expect(reindexProgress.increment).toHaveBeenCalledTimes(3);
+    expect(reindexProgress.increment).toHaveBeenCalledWith(WORKSPACE_ID);
+    // Cleared exactly once on completion.
+    expect(reindexProgress.clear).toHaveBeenCalledTimes(1);
+    expect(reindexProgress.clear).toHaveBeenCalledWith(WORKSPACE_ID);
+  });
+
+  it('counts a handled (non-fatal) per-page failure as processed', async () => {
+    const { service, reindexProgress } = makeService(['p1', 'p2', 'p3']);
+    // No statusCode -> non-fatal -> isolate and continue; each counts as done.
+    jest.spyOn(service, 'reindexPage').mockRejectedValue(new Error('boom'));
+
+    await service.reindexWorkspace(WORKSPACE_ID);
+
+    expect(reindexProgress.increment).toHaveBeenCalledTimes(3);
+    expect(reindexProgress.clear).toHaveBeenCalledTimes(1);
+  });
+
+  it('clears progress in finally even when a FATAL provider error aborts the batch', async () => {
+    const { service, reindexProgress } = makeService(['p1', 'p2', 'p3']);
+    // A 401 aborts on the first page (re-thrown) — the finally must still clear.
+    jest
+      .spyOn(service, 'reindexPage')
+      .mockRejectedValue({ statusCode: 401, message: 'User not found' });
+
+    await expect(service.reindexWorkspace(WORKSPACE_ID)).rejects.toMatchObject({
+      statusCode: 401,
+    });
+
+    expect(reindexProgress.start).toHaveBeenCalledWith(WORKSPACE_ID, 3);
+    // Aborted page is NOT counted as processed.
+    expect(reindexProgress.increment).not.toHaveBeenCalled();
+    // But progress is still cleared so the run never gets stuck.
+    expect(reindexProgress.clear).toHaveBeenCalledTimes(1);
+  });
+
+  it('clears the enqueue-seeded progress on an unconfigured early return', async () => {
+    const { service, aiService, reindexProgress } = makeService();
+    // Embeddings not configured: reindexWorkspace returns early WITHOUT starting
+    // a fresh record, but the finally must still clear the enqueue-time seed.
+    aiService.getEmbeddingModel = jest
+      .fn()
+      .mockRejectedValue(new AiEmbeddingNotConfiguredException());
+
+    await expect(
+      service.reindexWorkspace(WORKSPACE_ID),
+    ).resolves.toBeUndefined();
+
+    expect(reindexProgress.start).not.toHaveBeenCalled();
+    expect(reindexProgress.clear).toHaveBeenCalledTimes(1);
+    expect(reindexProgress.clear).toHaveBeenCalledWith(WORKSPACE_ID);
+  });
+});

apps/server/src/core/ai-chat/embedding/embedding-indexer.service.ts

@@ -9,6 +9,7 @@ import { KyselyDB } from '@docmost/db/types/kysely.types';
 import { InjectKysely } from 'nestjs-kysely';
 import { executeTx } from '@docmost/db/utils';
 import { AiService } from '../../../integrations/ai/ai.service';
+import { EmbeddingReindexProgressService } from '../../../integrations/ai/embedding-reindex-progress.service';
 import { AiEmbeddingNotConfiguredException } from '../../../integrations/ai/ai-embedding-not-configured.exception';
 import {
   describeProviderError,
@@ -48,6 +49,7 @@ export class EmbeddingIndexerService {
     private readonly pageRepo: PageRepo,
     private readonly pageEmbeddingRepo: PageEmbeddingRepo,
     private readonly aiService: AiService,
+    private readonly reindexProgress: EmbeddingReindexProgressService,
     @InjectKysely() private readonly db: KyselyDB,
   ) {}
 
@@ -183,7 +185,19 @@ export class EmbeddingIndexerService {
   }
 
   /**
-   * (Re)build embeddings for EVERY non-deleted page in a workspace. Used by the
+   * (Re)build embeddings for the EMBEDDABLE page set of a workspace — the same
+   * set countEmbeddablePages counts (via getEmbeddablePageIds): non-deleted pages
+   * that qualify under any of the three clauses of `embeddablePredicate` —
+   * non-empty textContent, OR an empty/null textContent whose ProseMirror
+   * `content` JSON has at least one text node (`"type":"text"`) that `jsonToText`
+   * can extract, OR an already-stored (non-deleted) embedding row — NOT every
+   * non-deleted page. Iterating this set keeps the live `total` equal to the
+   * steady-state denominator, so the progress counter climbs 0 -> total and
+   * matches the before/after DB coverage exactly. A page with truly no
+   * extractable text (empty textContent AND content with only non-text/atom
+   * nodes such as math) is correctly skipped (reindexPage no-ops on it); a page
+   * that lost its text but still has stale embeddings stays in the set (the
+   * EXISTS clause) so it is visited and its stale rows are cleared. Used by the
    * bulk reindex (WORKSPACE_CREATE_EMBEDDINGS, fired when AI Search is enabled
    * and by the manual "Reindex now" action).
    *
@@ -194,69 +208,99 @@ export class EmbeddingIndexerService {
    * the batch.
    */
   async reindexWorkspace(workspaceId: string): Promise<void> {
+    // The whole run is wrapped so the per-workspace progress record is ALWAYS
+    // cleared in the finally — on success, on a fatal-provider abort, on an
+    // unconfigured early-return, or on any unexpected throw — so a failed run
+    // never leaves a stuck "reindexing" state (the status then falls back to the
+    // steady-state DB coverage count). A placeholder record may already exist
+    // (seeded at enqueue time); the finally cleans that too.
     try {
-      await this.aiService.getEmbeddingModel(workspaceId);
-    } catch (err) {
-      if (err instanceof AiEmbeddingNotConfiguredException) {
-        this.logger.log(
-          `reindexWorkspace: embeddings not configured for workspace ${workspaceId}, skipping`,
-        );
-        return;
-      }
-      throw err;
-    }
-
-    const pageIds = await this.pageRepo.getIdsByWorkspace(workspaceId);
-    const total = pageIds.length;
-    const startedAt = Date.now();
-    this.logger.log(
-      `reindexWorkspace: starting reindex of ${total} page(s) for workspace ${workspaceId}`,
-    );
-
-    let failed = 0;
-    for (let i = 0; i < total; i++) {
-      const pageId = pageIds[i];
-      const position = i + 1;
-      // Log BEFORE the await: if the embedding call hangs, this is the last line
-      // in the log and it names the exact page that is stuck.
-      this.logger.log(
-        `reindexWorkspace: [${position}/${total}] indexing page ${pageId} (workspace ${workspaceId})`,
-      );
-      const pageStartedAt = Date.now();
       try {
-        await this.reindexPage(pageId);
-        const elapsed = Date.now() - pageStartedAt;
-        if (elapsed >= SLOW_PAGE_MS) {
-          this.logger.warn(
-            `reindexWorkspace: [${position}/${total}] page ${pageId} took ${elapsed}ms`,
-          );
-        }
+        await this.aiService.getEmbeddingModel(workspaceId);
       } catch (err) {
-        // A fatal provider error (invalid/missing key, no credits) recurs
-        // identically on EVERY remaining page. Abort the whole batch instead of
-        // issuing hundreds of doomed requests against the provider.
-        if (isFatalProviderError(err)) {
-          this.logger.error(
-            `reindexWorkspace: aborting at [${position}/${total}] for workspace ` +
-              `${workspaceId} — fatal provider error, remaining pages would fail ` +
-              `identically: ${describeProviderError(err)}`,
+        if (err instanceof AiEmbeddingNotConfiguredException) {
+          this.logger.log(
+            `reindexWorkspace: embeddings not configured for workspace ${workspaceId}, skipping`,
           );
-          throw err;
+          return;
         }
-        // Per-page isolation: one non-fatal failure (incl. an embedding timeout)
-        // must not abort the whole batch.
-        failed++;
-        this.logger.error(
-          `reindexWorkspace: [${position}/${total}] failed to reindex page ${pageId} ` +
-            `after ${Date.now() - pageStartedAt}ms: ${describeProviderError(err)}`,
-        );
+        throw err;
       }
-    }
 
-    this.logger.log(
-      `reindexWorkspace: done for workspace ${workspaceId}: ` +
-        `${total - failed}/${total} indexed, ${failed} failed in ${Date.now() - startedAt}ms`,
-    );
+      // Iterate the EMBEDDABLE set (same three-clause predicate as
+      // countEmbeddablePages), NOT every non-deleted page: this makes `total`
+      // here equal the steady-state denominator, so the live counter climbs
+      // 0 -> total and matches the before/after DB count exactly (no
+      // 478 -> 500 -> 478 denominator jump). Pages whose text lives in the
+      // ProseMirror `content` JSON (a text node) even with empty text_content ARE
+      // in this set (the content-JSON clause) and get embedded; a page with no
+      // extractable text at all is correctly skipped — reindexPage no-ops on it —
+      // and a page that lost its text but still has stale embeddings IS in this
+      // set (the EXISTS clause) so it is still visited and its stale rows cleared.
+      const pageIds = await this.pageRepo.getEmbeddablePageIds(workspaceId);
+      const total = pageIds.length;
+      const startedAt = Date.now();
+      // Publish the live run progress over this same set (done reset to 0). The
+      // counter increments once per iterated page and reaches exactly `total`,
+      // which equals countEmbeddablePages — the steady-state denominator.
+      await this.reindexProgress.start(workspaceId, total);
+      this.logger.log(
+        `reindexWorkspace: starting reindex of ${total} page(s) for workspace ${workspaceId}`,
+      );
+
+      let failed = 0;
+      for (let i = 0; i < total; i++) {
+        const pageId = pageIds[i];
+        const position = i + 1;
+        // Log BEFORE the await: if the embedding call hangs, this is the last line
+        // in the log and it names the exact page that is stuck.
+        this.logger.log(
+          `reindexWorkspace: [${position}/${total}] indexing page ${pageId} (workspace ${workspaceId})`,
+        );
+        const pageStartedAt = Date.now();
+        try {
+          await this.reindexPage(pageId);
+          // Count this page as processed (matches the [position/total] log).
+          await this.reindexProgress.increment(workspaceId);
+          const elapsed = Date.now() - pageStartedAt;
+          if (elapsed >= SLOW_PAGE_MS) {
+            this.logger.warn(
+              `reindexWorkspace: [${position}/${total}] page ${pageId} took ${elapsed}ms`,
+            );
+          }
+        } catch (err) {
+          // A fatal provider error (invalid/missing key, no credits) recurs
+          // identically on EVERY remaining page. Abort the whole batch instead of
+          // issuing hundreds of doomed requests against the provider. Do NOT count
+          // it as processed — the run aborts here (the finally clears progress).
+          if (isFatalProviderError(err)) {
+            this.logger.error(
+              `reindexWorkspace: aborting at [${position}/${total}] for workspace ` +
+                `${workspaceId} — fatal provider error, remaining pages would fail ` +
+                `identically: ${describeProviderError(err)}`,
+            );
+            throw err;
+          }
+          // Per-page isolation: one non-fatal failure (incl. an embedding timeout)
+          // must not abort the whole batch. A handled failure still advances the
+          // counter (matches the [position/total] log, so done reaches total).
+          failed++;
+          await this.reindexProgress.increment(workspaceId);
+          this.logger.error(
+            `reindexWorkspace: [${position}/${total}] failed to reindex page ${pageId} ` +
+              `after ${Date.now() - pageStartedAt}ms: ${describeProviderError(err)}`,
+          );
+        }
+      }
+
+      this.logger.log(
+        `reindexWorkspace: done for workspace ${workspaceId}: ` +
+          `${total - failed}/${total} indexed, ${failed} failed in ${Date.now() - startedAt}ms`,
+      );
+    } finally {
+      // Always remove the progress record so the status reverts to the DB count.
+      await this.reindexProgress.clear(workspaceId);
+    }
   }
 
   /** Purge ALL embeddings for a workspace (WORKSPACE_DELETE_EMBEDDINGS). */

apps/server/src/core/ai-chat/external-mcp/mcp-clients.service.spec.ts

@@ -0,0 +1,166 @@
+import { McpClientsService } from './mcp-clients.service';
+
+/**
+ * Unit tests for the two security-critical surfaces of McpClientsService that the
+ * sibling specs (ssrf-guard / validate-resolved-addresses / lease) do NOT cover:
+ *
+ *  1. `decryptHeaders` (private) — FAIL-OPEN behavior. A decrypt/parse failure
+ *     (e.g. APP_SECRET rotated, tampered blob) must NEVER throw and must NEVER
+ *     log the blob: it returns `undefined` so the connect proceeds WITHOUT the
+ *     now-unreadable auth headers (which then 401s and the server is skipped),
+ *     rather than crashing the whole turn.
+ *
+ *  2. `this.guardedFetch` (private, bound to the SSRF-pinned dispatcher) — the
+ *     per-request DNS-rebinding guard. A blocked host (private/loopback/metadata
+ *     IP literal, or an unparseable URL) must REJECT before any socket is opened;
+ *     a public host is allowed through to the real `fetch` with the pinned
+ *     dispatcher attached.
+ *
+ * No network and no DB: the repo + secretBox deps are stubbed, and global `fetch`
+ * is mocked for the single allow-path assertion.
+ */
+
+// Build the service with a SecretBoxService stub whose decryptSecret is supplied
+// per-test. The repo dep is unused by the methods under test.
+function buildService(decryptSecret: (blob: string) => string) {
+  const secretBox = { decryptSecret: jest.fn(decryptSecret) };
+  const service = new McpClientsService({} as never, secretBox as never);
+  return { service, secretBox };
+}
+
+describe('McpClientsService.decryptHeaders', () => {
+  // Reach the private method via the as-any pattern common in these NestJS specs.
+  const callDecrypt = (
+    service: McpClientsService,
+    blob: string | null,
+  ): Record<string, string> | undefined =>
+    (
+      service as unknown as {
+        decryptHeaders: (b: string | null) => Record<string, string> | undefined;
+      }
+    ).decryptHeaders(blob);
+
+  it('returns undefined for a null blob without decrypting', () => {
+    const { service, secretBox } = buildService(() => '{}');
+    expect(callDecrypt(service, null)).toBeUndefined();
+    expect(secretBox.decryptSecret).not.toHaveBeenCalled();
+  });
+
+  it('decrypts a valid blob and keeps only string-valued headers', () => {
+    const { service } = buildService(() =>
+      JSON.stringify({
+        Authorization: 'Bearer abc',
+        'X-Api-Key': 'k',
+        // Non-string values must be dropped, not coerced.
+        count: 5,
+        flag: true,
+        nested: { a: 1 },
+      }),
+    );
+    expect(callDecrypt(service, 'cipher')).toEqual({
+      Authorization: 'Bearer abc',
+      'X-Api-Key': 'k',
+    });
+  });
+
+  it('returns undefined when the decrypted object has no string headers', () => {
+    const { service } = buildService(() => JSON.stringify({ count: 5 }));
+    // No usable headers -> undefined (connect with no auth header), not {}.
+    expect(callDecrypt(service, 'cipher')).toBeUndefined();
+  });
+
+  it('FAILS OPEN: a decrypt error returns undefined instead of throwing', () => {
+    const { service } = buildService(() => {
+      throw new Error('Failed to decrypt secret — APP_SECRET may have changed');
+    });
+    const warnSpy = jest
+      .spyOn(
+        (service as unknown as { logger: { warn: (...a: unknown[]) => void } })
+          .logger,
+        'warn',
+      )
+      .mockImplementation(() => undefined);
+
+    let result: unknown;
+    expect(() => {
+      result = callDecrypt(service, 'tampered-blob');
+    }).not.toThrow();
+    expect(result).toBeUndefined();
+    // It warns (so ops sees degradation) but never logs the blob itself.
+    expect(warnSpy).toHaveBeenCalledTimes(1);
+    expect(String(warnSpy.mock.calls[0]?.[0])).not.toContain('tampered-blob');
+  });
+
+  it('FAILS OPEN: malformed JSON (decrypts to non-JSON) returns undefined', () => {
+    const { service } = buildService(() => 'not-json{');
+    jest
+      .spyOn(
+        (service as unknown as { logger: { warn: (...a: unknown[]) => void } })
+          .logger,
+        'warn',
+      )
+      .mockImplementation(() => undefined);
+    expect(callDecrypt(service, 'cipher')).toBeUndefined();
+  });
+});
+
+describe('McpClientsService.guardedFetch (SSRF per-request guard)', () => {
+  // The bound guardedFetch closure lives on the instance as a private field.
+  const guardedFetchOf = (service: McpClientsService) =>
+    (service as unknown as { guardedFetch: typeof fetch }).guardedFetch;
+
+  let fetchSpy: jest.SpiedFunction<typeof fetch>;
+
+  beforeEach(() => {
+    // Any reachable real fetch would be a network call; assert per-test that the
+    // blocked paths never reach it, and stub a Response for the allow path.
+    fetchSpy = jest
+      .spyOn(global, 'fetch')
+      .mockResolvedValue(new Response('ok', { status: 200 }));
+  });
+
+  afterEach(() => {
+    jest.restoreAllMocks();
+  });
+
+  const blocked: Array<[string, string]> = [
+    ['loopback IPv4', 'http://127.0.0.1/mcp'],
+    ['private 10/8', 'http://10.0.0.5/mcp'],
+    ['private 192.168/16', 'http://192.168.1.1/mcp'],
+    ['cloud metadata link-local', 'http://169.254.169.254/latest/meta-data/'],
+    ['loopback IPv6 (bracketed)', 'http://[::1]:8080/mcp'],
+  ];
+
+  it.each(blocked)(
+    'rejects a request to %s without opening a socket',
+    async (_label, url) => {
+      const { service } = buildService(() => '{}');
+      await expect(guardedFetchOf(service)(url)).rejects.toThrow(
+        /blocked request/,
+      );
+      expect(fetchSpy).not.toHaveBeenCalled();
+    },
+  );
+
+  it('rejects an unparseable URL as a blocked request', async () => {
+    const { service } = buildService(() => '{}');
+    await expect(
+      guardedFetchOf(service)('::: not a url :::'),
+    ).rejects.toThrow('blocked request: invalid URL');
+    expect(fetchSpy).not.toHaveBeenCalled();
+  });
+
+  it('allows a public IP literal and forwards through the pinned dispatcher', async () => {
+    const { service } = buildService(() => '{}');
+    const res = await guardedFetchOf(service)('http://8.8.8.8/mcp');
+
+    expect(res.status).toBe(200);
+    expect(fetchSpy).toHaveBeenCalledTimes(1);
+    // The init MUST carry the SSRF-pinned undici dispatcher (the rebinding pin);
+    // dropping it would let undici do a second, unchecked DNS resolution.
+    const init = fetchSpy.mock.calls[0][1] as RequestInit & {
+      dispatcher?: unknown;
+    };
+    expect(init.dispatcher).toBeDefined();
+  });
+});

apps/server/src/core/ai-chat/tools/shared-tool-specs.contract.spec.ts

@@ -0,0 +1,124 @@
+import { z } from 'zod';
+import { AiChatToolsService } from './ai-chat-tools.service';
+import * as loader from './docmost-client.loader';
+import type { DocmostClientLike } from './docmost-client.loader';
+// The real zod-agnostic registry, imported from source so the contract is checked
+// against exactly what the @docmost/mcp package ships (no hand-stub).
+import { SHARED_TOOL_SPECS } from '../../../../../../packages/mcp/src/tool-specs';
+
+/**
+ * CONTRACT: SHARED_TOOL_SPECS <-> in-app tool wiring parity.
+ *
+ * `packages/mcp/src/tool-specs.ts` is the single source of truth for the tools
+ * that are intentionally IDENTICAL across the standalone MCP server (zod v3) and
+ * the in-app AI-SDK service (zod v4). The in-app service builds each one via
+ * `sharedTool(sharedToolSpecs.<key>, execute)`, keyed by the spec's `inAppKey`.
+ *
+ * This test fails the build if a spec is added to the registry but never wired
+ * in-app, if an `inAppKey` is renamed without updating the service, if the
+ * description drifts between the registry and the exposed tool, if the
+ * snake_case `mcpName` <-> camelCase `inAppKey` convention is broken, or if the
+ * exposed tool's input-schema keys diverge from the spec's `buildShape`.
+ *
+ * It does NOT need @docmost/mcp built: the registry is imported from TS source,
+ * and the ESM loader is mocked so `forUser()` never dynamically imports the
+ * package.
+ */
+describe('SHARED_TOOL_SPECS contract parity', () => {
+  // Empty fake client: no tool is executed here — every assertion is on tool
+  // presence / metadata / schema, so the client methods are never called.
+  const fakeClient: Partial<DocmostClientLike> = {};
+  const tokenServiceStub = {
+    generateAccessToken: jest.fn().mockResolvedValue('access-token'),
+    generateCollabToken: jest.fn().mockResolvedValue('collab-token'),
+  };
+
+  let tools: Record<string, unknown>;
+
+  beforeAll(async () => {
+    jest.spyOn(loader, 'loadDocmostMcp').mockResolvedValue({
+      DocmostClient: function () {
+        return fakeClient as DocmostClientLike;
+      } as unknown as loader.DocmostClientCtor,
+      // Feed the service the SAME registry this test asserts against.
+      sharedToolSpecs: SHARED_TOOL_SPECS as unknown as Record<
+        string,
+        loader.SharedToolSpec
+      >,
+    });
+    const service = new AiChatToolsService(
+      tokenServiceStub as never,
+      {} as never,
+      {} as never,
+      {} as never,
+      {} as never,
+      { asSink: () => ({ put: jest.fn(), has: jest.fn(), evict: jest.fn() }) } as never,
+    );
+    tools = (await service.forUser(
+      { id: 'user-1', email: 'u@example.com', workspaceId: 'ws-1' } as never,
+      'session-1',
+      'ws-1',
+      'chat-1',
+    )) as unknown as Record<string, unknown>;
+  });
+
+  afterAll(() => jest.restoreAllMocks());
+
+  // camelCase -> snake_case, matching the registry's mcpName convention.
+  const toSnake = (s: string) =>
+    s.replace(/[A-Z]/g, (c) => `_${c.toLowerCase()}`);
+
+  // Type as the (optional-buildShape) SharedToolSpec; the `satisfies` literal
+  // above otherwise narrows to a union where some members lack buildShape.
+  const specEntries = Object.entries(SHARED_TOOL_SPECS) as Array<
+    [string, loader.SharedToolSpec]
+  >;
+
+  // Sanity: the registry is non-empty, so the per-spec table below is not vacuous.
+  it('registry is non-empty', () => {
+    expect(specEntries.length).toBeGreaterThan(0);
+  });
+
+  describe.each(specEntries)('spec "%s"', (registryKey, spec) => {
+    it('registry key equals its inAppKey', () => {
+      // The service indexes the registry by property name; a key != inAppKey
+      // would wire the wrong (or no) tool.
+      expect(spec.inAppKey).toBe(registryKey);
+    });
+
+    it('mcpName is the snake_case form of inAppKey', () => {
+      expect(spec.mcpName).toBe(toSnake(spec.inAppKey));
+    });
+
+    it('is exposed in-app under its inAppKey', () => {
+      // Fails if a spec is added to the registry but never wired in forUser().
+      expect(tools[spec.inAppKey]).toBeDefined();
+    });
+
+    it("exposed tool's description matches the registry description", () => {
+      const tool = tools[spec.inAppKey] as { description: string };
+      expect(tool.description).toBe(spec.description);
+    });
+
+    it("exposed tool's input-schema keys match buildShape (incl. required)", () => {
+      const tool = tools[spec.inAppKey] as {
+        inputSchema: { jsonSchema: { properties?: Record<string, unknown>; required?: string[] } };
+      };
+      const json = tool.inputSchema.jsonSchema;
+      const actualKeys = Object.keys(json.properties ?? {}).sort();
+
+      // Derive the spec's declared shape with THIS layer's zod (v4) — the same
+      // call the service makes — then compare key sets and required-ness.
+      const shape = spec.buildShape ? spec.buildShape(z) : {};
+      const expectedKeys = Object.keys(shape).sort();
+      expect(actualKeys).toEqual(expectedKeys);
+
+      // A non-.optional() field must surface as required in the advertised schema.
+      const expectedRequired = Object.entries(shape)
+        .filter(([, field]) => !(field as z.ZodTypeAny).isOptional?.())
+        .map(([k]) => k)
+        .sort();
+      expect((json.required ?? []).slice().sort()).toEqual(expectedRequired);
+    });
+  });
+});

apps/server/src/core/share/share-spoiler-keep.spec.ts

@@ -1,129 +0,0 @@
-import { ShareService } from './share.service';
-
-// Sibling of share-comment-strip.spec.ts. The public-share sanitizer strips ONLY
-// `comment` marks (internal-team metadata) via removeMarkTypeFromDoc(doc,
-// 'comment'). The `spoiler` mark is legitimate authored content (hidden text the
-// reader clicks to reveal) and MUST survive the share-strip — otherwise public
-// readers would see the secret in plain text or lose it entirely.
-//
-// We drive the SAME real seam the comment-strip test uses:
-// updatePublicAttachments -> prepareContentForShare -> removeMarkTypeFromDoc.
-
-const WS = 'ws-1';
-const PAGE = 'page-1';
-
-function buildService() {
-  const shareRepo = { findById: jest.fn() };
-  const pageRepo = { findById: jest.fn() };
-  const pagePermissionRepo = {
-    hasRestrictedAncestor: jest.fn(async () => false),
-  };
-  const tokenService = {
-    generateAttachmentToken: jest.fn(async () => 'tok'),
-  };
-  const workspaceRepo = {
-    findById: jest.fn(async () => ({ id: WS, settings: { htmlEmbed: true } })),
-  };
-
-  return new ShareService(
-    shareRepo as any,
-    pageRepo as any,
-    pagePermissionRepo as any,
-    {} as any, // db (unused on this path)
-    tokenService as any,
-    {} as any, // transclusionService (unused)
-    workspaceRepo as any,
-  );
-}
-
-// Text carrying a `spoiler` mark (no attributes; revealed state is UI-only).
-function spoilerText(text: string) {
-  return {
-    type: 'text',
-    text,
-    marks: [{ type: 'spoiler' }],
-  };
-}
-
-// Text carrying a `comment` mark with an id (the thing that DOES get stripped).
-function commentedText(text: string, commentId: string) {
-  return {
-    type: 'text',
-    text,
-    marks: [{ type: 'comment', attrs: { commentId, resolved: false } }],
-  };
-}
-
-async function sanitize(content: any) {
-  const service = buildService();
-  return service.updatePublicAttachments({
-    id: PAGE,
-    workspaceId: WS,
-    content,
-  } as any);
-}
-
-function countMarks(doc: any, type: string): number {
-  let count = 0;
-  const walk = (node: any) => {
-    if (!node || typeof node !== 'object') return;
-    if (Array.isArray(node.marks)) {
-      for (const mark of node.marks) {
-        if (mark?.type === type) count++;
-      }
-    }
-    if (Array.isArray(node.content)) node.content.forEach(walk);
-  };
-  walk(doc);
-  return count;
-}
-
-describe('ShareService keeps spoiler marks on public shares (real code)', () => {
-  it('does NOT strip a spoiler mark', async () => {
-    const content = {
-      type: 'doc',
-      content: [
-        {
-          type: 'paragraph',
-          content: [{ type: 'text', text: 'visible ' }, spoilerText('hidden')],
-        },
-      ],
-    };
-
-    expect(countMarks(content, 'spoiler')).toBe(1);
-
-    const out = await sanitize(content);
-
-    // The spoiler mark survives the share-strip.
-    expect(countMarks(out, 'spoiler')).toBe(1);
-    expect(JSON.stringify(out)).toContain('hidden');
-  });
-
-  it('strips comment marks but keeps spoiler marks in the same doc', async () => {
-    const content = {
-      type: 'doc',
-      content: [
-        {
-          type: 'paragraph',
-          content: [
-            commentedText('reviewed', 'cmt-1'),
-            { type: 'text', text: ' and ' },
-            spoilerText('secret'),
-          ],
-        },
-      ],
-    };
-
-    expect(countMarks(content, 'comment')).toBe(1);
-    expect(countMarks(content, 'spoiler')).toBe(1);
-
-    const out = await sanitize(content);
-
-    // comment is removed, spoiler is preserved.
-    expect(countMarks(out, 'comment')).toBe(0);
-    expect(countMarks(out, 'spoiler')).toBe(1);
-    const serialized = JSON.stringify(out);
-    expect(serialized).not.toContain('cmt-1');
-    expect(serialized).toContain('secret');
-  });
-});

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

@@ -0,0 +1,167 @@
+import { PageRepo } from './page.repo';
+import {
+  DummyDriver,
+  Kysely,
+  PostgresAdapter,
+  PostgresIntrospector,
+  PostgresQueryCompiler,
+} from 'kysely';
+
+/**
+ * F6 regression guard for the embeddable-page predicate.
+ *
+ * The predicate is shared by `countEmbeddablePages` (the "Indexed N of M" coverage
+ * denominator) and `getEmbeddablePageIds` (the exact set a full reindex iterates).
+ * It MUST select pages whose `text_content` was never backfilled (null/empty) but
+ * whose ProseMirror `content` JSON still carries body text — `reindexPage` builds
+ * its chunks straight from `content`, so without a content clause such a page is
+ * silently SKIPPED by a mass reindex even though it is fully embeddable.
+ *
+ * The content clause keys on the structural text-node marker `"type":"text"`, NOT
+ * a bare `"text":` key. The bare key also appears as the `attrs.text` of atom
+ * nodes that carry NO extractable text — notably math (`mathBlock`/`mathInline`),
+ * whose LaTeX lives in `attrs.text` and has no `generateText` serializer. A
+ * math-ONLY page therefore yields empty `text_content` and zero embeddings; if the
+ * predicate matched its `attrs.text` it would land in the denominator but
+ * `reindexPage` would no-op on it, pinning "Indexed N of M" below 100% forever —
+ * the exact bug this feature fixes. The `"type":"text"` marker matches only real
+ * text nodes (what `jsonToText` extracts), keeping the predicate consistent with
+ * what gets indexed.
+ *
+ * There is no real Postgres here: a recording Kysely (DummyDriver wired to the
+ * Postgres query compiler) compiles the queries to SQL so we can assert the WHERE
+ * predicate ORs in the narrowed content clause alongside the existing text_content
+ * and stored-embeddings clauses — and that BOTH callers compile the identical
+ * clause (denominator and reindex set can never diverge).
+ */
+function makeRecordingDb() {
+  const sqls: string[] = [];
+  const db = new Kysely<any>({
+    dialect: {
+      createAdapter: () => new PostgresAdapter(),
+      createDriver: () =>
+        new (class extends DummyDriver {
+          async acquireConnection() {
+            return {
+              executeQuery: async (compiled: { sql: string }) => {
+                sqls.push(compiled.sql);
+                return { rows: [] };
+              },
+              // eslint-disable-next-line @typescript-eslint/no-empty-function
+              streamQuery: async function* () {},
+            } as any;
+          }
+        })(),
+      createIntrospector: (d: Kysely<any>) => new PostgresIntrospector(d),
+      createQueryCompiler: () => new PostgresQueryCompiler(),
+    },
+  });
+  return { db, sqls };
+}
+
+// The narrowed content clause, as it appears in the compiled SQL. Keying on the
+// structural `"type":"text"` marker (not a bare `"text":` key) is what excludes
+// math-only pages whose only `"text"` key is the atom node's `attrs.text`.
+const NARROWED_CLAUSE = `"type"[[:space:]]*:[[:space:]]*"text"`;
+const BARE_TEXT_KEY = `"text"[[:space:]]*:`;
+
+describe('PageRepo embeddable predicate — content-bearing pages (F6)', () => {
+  it('selects content-bearing pages via the narrowed "type":"text" node marker', async () => {
+    const { db, sqls } = makeRecordingDb();
+    const repo = new PageRepo(db as any, {} as any, { emit: jest.fn() } as any);
+
+    await repo.getEmbeddablePageIds('ws-1');
+
+    expect(sqls).toHaveLength(1);
+    const sql = sqls[0];
+
+    // Clause 1 (existing): pages with extractable text_content.
+    expect(sql).toContain('text_content');
+    // Clause 3 (the F6 fix, now narrowed): a page whose content JSON carries a
+    // real text node is selected even when text_content is null/empty, so a full
+    // reindex visits it instead of silently skipping it.
+    expect(sql).toContain('content::text');
+    expect(sql).toContain(NARROWED_CLAUSE);
+    // It must NOT use the old bare `"text":` key, which also matches the
+    // `attrs.text` of math-only atom pages (false-positive denominator inflation).
+    expect(sql).not.toContain(BARE_TEXT_KEY);
+    // Clause 2 (existing): pages that already have stored embeddings stay in the
+    // set so a reindex can clear their stale rows.
+    expect(sql.toLowerCase()).toContain('embeddings');
+  });
+
+  it('countEmbeddablePages compiles the SAME narrowed clause as getEmbeddablePageIds', async () => {
+    // Consistency is the core requirement: the denominator (countEmbeddablePages)
+    // and the reindex set (getEmbeddablePageIds) MUST share the identical
+    // predicate, else the live "done" counter and the steady-state total diverge.
+    const { db, sqls } = makeRecordingDb();
+    const repo = new PageRepo(db as any, {} as any, { emit: jest.fn() } as any);
+
+    await repo.countEmbeddablePages('ws-1');
+    await repo.getEmbeddablePageIds('ws-1');
+
+    expect(sqls).toHaveLength(2);
+    const [countSql, idsSql] = sqls;
+
+    // Both carry the narrowed content clause...
+    expect(countSql).toContain(NARROWED_CLAUSE);
+    expect(idsSql).toContain(NARROWED_CLAUSE);
+    // ...neither carries the bare key...
+    expect(countSql).not.toContain(BARE_TEXT_KEY);
+    expect(idsSql).not.toContain(BARE_TEXT_KEY);
+    // ...and the full OR predicate (text_content + content node + embeddings
+    // EXISTS) is byte-identical between the two queries, so they can't drift.
+    const where = (s: string) => s.slice(s.indexOf('where'));
+    expect(where(countSql)).toEqual(where(idsSql));
+  });
+
+  it('the content regex matches a text-bearing doc but NOT a math-only doc', () => {
+    // Semantic check of the predicate against sample `content::text` payloads.
+    // Note: `jsonb::text` is NOT identical to JSON.stringify — Postgres renders a
+    // space after each colon (`"type": "text"`), which is exactly why the POSIX
+    // clause uses `[[:space:]]*`. The clause `"type"[[:space:]]*:[[:space:]]*"text"`
+    // maps to the JS regex below (`[[:space:]]` -> `\s`, tolerating both forms);
+    // we evaluate it the way Postgres would.
+    const re = /"type"\s*:\s*"text"/;
+
+    // A real paragraph with a text node -> embeddable.
+    const textDoc = JSON.stringify({
+      type: 'doc',
+      content: [
+        {
+          type: 'paragraph',
+          content: [{ type: 'text', text: 'hello world' }],
+        },
+      ],
+    });
+    // A doc whose ONLY node is a math atom. Its LaTeX is in `attrs.text`, there is
+    // no text node, and `jsonToText`/`generateText` has no serializer for it -> it
+    // yields empty text_content and zero embeddings, so it must NOT qualify.
+    const mathOnlyDoc = JSON.stringify({
+      type: 'doc',
+      content: [
+        { type: 'mathBlock', attrs: { text: 'E = mc^2' } },
+        { type: 'mathInline', attrs: { text: '\\alpha' } },
+      ],
+    });
+    // An empty doc has no text node either.
+    const emptyDoc = JSON.stringify({ type: 'doc', content: [] });
+
+    expect(re.test(textDoc)).toBe(true);
+    expect(re.test(mathOnlyDoc)).toBe(false);
+    expect(re.test(emptyDoc)).toBe(false);
+    // Sanity: the OLD bare-key regex WOULD have wrongly matched the math-only doc,
+    // which is precisely the false positive the narrowing removes.
+    expect(/"text"\s*:/.test(mathOnlyDoc)).toBe(true);
+
+    // A user literally TYPING `"type":"text"` in prose can't false-positive on an
+    // otherwise text-less page: in `content::text` the typed value's quotes are
+    // escaped (`\"type\":\"text\"`), so the literal-quote regex does not match the
+    // escaped form. (And such a page is a genuine text node anyway.)
+    const escapedLiteral = JSON.stringify({
+      type: 'doc',
+      content: [{ type: 'someAtom', attrs: { note: '"type":"text"' } }],
+    });
+    expect(re.test(escapedLiteral)).toBe(false);
+  });
+});

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

@@ -12,6 +12,7 @@ import { executeWithCursorPagination } from '@docmost/db/pagination/cursor-pagin
 import { validate as isValidUUID } from 'uuid';
 import { ExpressionBuilder, sql } from 'kysely';
 import { DB } from '@docmost/db/types/db';
+import { DbInterface } from '@docmost/db/types/db.interface';
 import { jsonArrayFrom, jsonObjectFrom } from 'kysely/helpers/postgres';
 import { SpaceMemberRepo } from '@docmost/db/repos/space/space-member.repo';
 import { EventEmitter2 } from '@nestjs/event-emitter';
@@ -233,9 +234,9 @@ export class PageRepo {
    * text-less pages (which legitimately store zero embeddings) don't keep the
    * bar below 100% forever.
    *
-   * A page qualifies if it has non-empty textContent OR already has stored
-   * embeddings. The second clause covers pages whose text the indexer extracted
-   * from the content JSON when textContent was null, and guarantees this total is
+   * A page qualifies if it has non-empty textContent, OR its content JSON has at
+   * least one text node (`"type":"text"`) when textContent was never backfilled,
+   * OR it already has stored embeddings. The last clause guarantees this total is
    * always >= countIndexedPages (the indexed count can never exceed it).
    */
   async countEmbeddablePages(workspaceId: string): Promise<number> {
@@ -243,37 +244,91 @@ export class PageRepo {
       .selectFrom('pages as p')
       .where('p.workspaceId', '=', workspaceId)
       .where('p.deletedAt', 'is', null)
-      .where((eb) =>
-        eb.or([
-          // Has extractable body text. The regex matches any non-whitespace
-          // character, mirroring the indexer's `text.trim().length === 0` check
-          // (raw SQL -> use the snake_case column name).
-          sql<boolean>`p.text_content ~ '[^[:space:]]'`,
-          // OR already has at least one (non-deleted) embedding row.
-          eb.exists(
-            eb
-              .selectFrom('pageEmbeddings as pe')
-              .select(sql`1`.as('one'))
-              .whereRef('pe.pageId', '=', 'p.id')
-              .where('pe.deletedAt', 'is', null),
-          ),
-        ]),
-      )
+      .where((eb) => this.embeddablePredicate(eb))
       .select((eb) => eb.fn.countAll().as('count'))
       .executeTakeFirst();
     return Number(row?.count ?? 0);
   }
 
   /**
-   * IDs of all non-deleted pages in a workspace. Used by the RAG bulk reindex to
-   * (re)build embeddings for every existing page.
+   * The "embeddable content" qualifying predicate, shared verbatim by
+   * countEmbeddablePages (the steady-state denominator) and getEmbeddablePageIds
+   * (the set the bulk reindex iterates). Both MUST use the exact same condition
+   * or the live total and steady-state total diverge — extracting it here is what
+   * guarantees that, replacing the previous hand-duplicated copy. Callers supply
+   * the trivial workspaceId/deletedAt filters inline; this returns only the
+   * non-trivial OR clause, evaluated against the `p` alias of `pages`.
+   *
+   * A page qualifies if it has non-empty textContent, OR its ProseMirror
+   * `content` JSON has at least one text node (`"type":"text"`) even though
+   * textContent was never backfilled, OR it already has a stored (non-deleted)
+   * embedding row.
    */
-  async getIdsByWorkspace(workspaceId: string): Promise<string[]> {
+  private embeddablePredicate(
+    eb: ExpressionBuilder<DbInterface & { p: DbInterface['pages'] }, 'p'>,
+  ) {
+    return eb.or([
+      // Has extractable body text. The regex matches any non-whitespace
+      // character, mirroring the indexer's `text.trim().length === 0` check
+      // (raw SQL -> use the snake_case column name).
+      sql<boolean>`p.text_content ~ '[^[:space:]]'`,
+      // OR the ProseMirror `content` JSON has at least one text node (`"type":
+      // "text"`) the indexer can extract, even when `text_content` is null/empty
+      // (never backfilled): `reindexPage` runs `jsonToText` (generateText) over
+      // `content`, which only emits the text of ProseMirror text nodes, so such a
+      // page IS embeddable and a full reindex MUST visit it (otherwise it is
+      // silently skipped). A text node always serialises as
+      // `{"type":"text","text":"..."}`, so we key on the structural `"type":
+      // "text"` marker — NOT a bare `"text":` key, which also appears as the
+      // `attrs.text` of atom nodes that carry NO extractable text (e.g. math
+      // `mathBlock`/`mathInline`, whose LaTeX lives in `attrs.text` and has no
+      // text serializer). A math-only page thus produces empty `text_content` and
+      // zero embeddings; matching its `attrs.text` here would wrongly inflate the
+      // denominator and keep "Indexed N of M" below 100% forever. An empty doc
+      // (no text nodes) has no `"type":"text"` and is correctly excluded. A user
+      // who literally types `"type":"text"` in their prose can't false-positive:
+      // in `content::text` that text value's quotes are escaped (`\"type\"...`),
+      // so the literal-quote regex won't match the escaped form (and such a page
+      // is a real text node anyway).
+      sql<boolean>`p.content::text ~ '"type"[[:space:]]*:[[:space:]]*"text"'`,
+      // OR already has at least one (non-deleted) embedding row.
+      eb.exists(
+        eb
+          .selectFrom('pageEmbeddings as pe')
+          .select(sql`1`.as('one'))
+          .whereRef('pe.pageId', '=', 'p.id')
+          .where('pe.deletedAt', 'is', null),
+      ),
+    ]);
+  }
+
+  /**
+   * IDs of the EMBEDDABLE page set for a workspace — the exact same set that
+   * `countEmbeddablePages` counts (a page qualifies if it has non-empty
+   * textContent, OR content JSON with at least one text node (`"type":"text"`)
+   * and an empty/null textContent, OR already has a stored embedding row). The
+   * bulk reindex
+   * iterates THIS set so the live "done" counter reaches exactly
+   * `countEmbeddablePages` (the steady-state denominator), instead of iterating
+   * every non-deleted page (which would push the denominator above the
+   * steady-state value mid-run).
+   *
+   * IMPORTANT: the qualifying WHERE is shared with `countEmbeddablePages` via the
+   * private `embeddablePredicate` helper, so the two can no longer drift — if the
+   * embeddable definition changes, change it once there and both stay in lockstep
+   * (else the live total and steady-state total diverge again). Dropping
+   * text-less pages is correct: `reindexPage` no-ops on
+   * a page with no extractable content anyway, and a page that lost its text but
+   * still has stale embeddings IS in this set (the EXISTS clause), so it is still
+   * visited and its stale rows are cleared.
+   */
+  async getEmbeddablePageIds(workspaceId: string): Promise<string[]> {
     const rows = await this.db
-      .selectFrom('pages')
-      .select('id')
-      .where('workspaceId', '=', workspaceId)
-      .where('deletedAt', 'is', null)
+      .selectFrom('pages as p')
+      .select('p.id')
+      .where('p.workspaceId', '=', workspaceId)
+      .where('p.deletedAt', 'is', null)
+      .where((eb) => this.embeddablePredicate(eb))
       .execute();
     return rows.map((r) => r.id);
   }

apps/server/src/integrations/ai/ai-settings.service.spec.ts

@@ -1,4 +1,12 @@
-import { parsePositiveInt } from './ai-settings.service';
+import { AiSettingsService, parsePositiveInt } from './ai-settings.service';
+import { WorkspaceRepo } from '@docmost/db/repos/workspace/workspace.repo';
+import { AiAgentRoleRepo } from '@docmost/db/repos/ai-agent-roles/ai-agent-roles.repo';
+import { AiProviderCredentialsRepo } from '@docmost/db/repos/ai-chat/ai-provider-credentials.repo';
+import { PageEmbeddingRepo } from '@docmost/db/repos/ai-chat/page-embedding.repo';
+import { PageRepo } from '@docmost/db/repos/page/page.repo';
+import { SecretBoxService } from '../crypto/secret-box';
+import { EmbeddingReindexProgressService } from './embedding-reindex-progress.service';
+import type { Queue } from 'bullmq';
 
 /**
  * Round-trip coercion for numeric `::text` provider settings (e.g.
@@ -41,3 +49,196 @@ describe('parsePositiveInt', () => {
     expect(parsePositiveInt(42)).toBe(42);
   });
 });
+
+/**
+ * getMasked must surface the LIVE reindex run progress while a reindex is active
+ * (so the "Indexed X of Y" counter can climb 0 -> total), and fall back to the
+ * steady-state DB coverage count (countIndexedPages / countEmbeddablePages) when
+ * no reindex is running. This is the server side of the fix for the counter that
+ * otherwise stays stuck at "478 of 478" the whole reindex.
+ */
+describe('AiSettingsService.getMasked reindex progress', () => {
+  const WORKSPACE_ID = 'ws-1';
+
+  function makeService() {
+    // No driver configured -> the credentials lookup is skipped, keeping the
+    // setup minimal; we only care about the indexed/total numbers here.
+    const workspaceRepo = {
+      findById: jest.fn().mockResolvedValue({ settings: {} }),
+    };
+    const aiAgentRoleRepo = {};
+    const aiProviderCredentialsRepo = { find: jest.fn() };
+    const pageEmbeddingRepo = {
+      countIndexedPages: jest.fn().mockResolvedValue(478),
+    };
+    const pageRepo = {
+      countEmbeddablePages: jest.fn().mockResolvedValue(478),
+    };
+    const secretBox = {};
+    const reindexProgress = {
+      get: jest.fn().mockResolvedValue(null),
+    };
+    const aiQueue = {};
+
+    const service = new AiSettingsService(
+      workspaceRepo as unknown as WorkspaceRepo,
+      aiAgentRoleRepo as unknown as AiAgentRoleRepo,
+      aiProviderCredentialsRepo as unknown as AiProviderCredentialsRepo,
+      pageEmbeddingRepo as unknown as PageEmbeddingRepo,
+      pageRepo as unknown as PageRepo,
+      secretBox as unknown as SecretBoxService,
+      reindexProgress as unknown as EmbeddingReindexProgressService,
+      aiQueue as unknown as Queue,
+    );
+    return { service, reindexProgress, pageEmbeddingRepo };
+  }
+
+  it('reports the live run numbers when a reindex progress record is active', async () => {
+    const { service, reindexProgress } = makeService();
+    // Use a progress.total (500) DISTINCT from the DB count (478) so the test
+    // actually pins the progress.total branch rather than coincidentally
+    // matching the DB fallback. With fix #1 the two sources agree in practice,
+    // but getMasked must still return progress.total when a record is active.
+    reindexProgress.get.mockResolvedValue({
+      total: 500,
+      done: 120,
+      startedAt: Date.now(),
+    });
+
+    const masked = await service.getMasked(WORKSPACE_ID);
+
+    expect(masked.indexedPages).toBe(120); // progress.done, not DB 478
+    expect(masked.totalPages).toBe(500); // progress.total, not DB 478
+    expect(masked.reindexing).toBe(true);
+  });
+
+  it('falls back to countIndexedPages when no reindex is active', async () => {
+    const { service, reindexProgress } = makeService();
+    reindexProgress.get.mockResolvedValue(null);
+
+    const masked = await service.getMasked(WORKSPACE_ID);
+
+    expect(masked.indexedPages).toBe(478);
+    expect(masked.totalPages).toBe(478);
+    expect(masked.reindexing).toBe(false);
+  });
+});
+
+/**
+ * reindex() must seed a live progress record (done=0) BEFORE enqueueing so the
+ * first status poll shows 0 — but ONLY when no run is already active, since
+ * aiQueue.add() de-duplicates a running reindex and a re-seed would reset the
+ * visible counter to 0 while the live worker keeps incrementing from its real
+ * position.
+ */
+describe('AiSettingsService.reindex progress seed', () => {
+  const WORKSPACE_ID = 'ws-1';
+
+  function makeService() {
+    const order: string[] = [];
+    const aiQueue = {
+      remove: jest.fn().mockResolvedValue(undefined),
+      add: jest.fn().mockImplementation(async () => {
+        order.push('add');
+      }),
+    };
+    const pageRepo = {
+      countEmbeddablePages: jest.fn().mockResolvedValue(478),
+    };
+    const reindexProgress = {
+      // Default: no active run -> seed should happen.
+      get: jest.fn().mockResolvedValue(null),
+      start: jest.fn().mockImplementation(async () => {
+        order.push('start');
+      }),
+      clear: jest.fn().mockResolvedValue(undefined),
+    };
+
+    const service = new AiSettingsService(
+      {} as unknown as WorkspaceRepo,
+      {} as unknown as AiAgentRoleRepo,
+      {} as unknown as AiProviderCredentialsRepo,
+      {} as unknown as PageEmbeddingRepo,
+      pageRepo as unknown as PageRepo,
+      {} as unknown as SecretBoxService,
+      reindexProgress as unknown as EmbeddingReindexProgressService,
+      aiQueue as unknown as Queue,
+    );
+    return { service, aiQueue, pageRepo, reindexProgress, order };
+  }
+
+  it('seeds progress (workspace, count) BEFORE enqueue when no run is active', async () => {
+    const { service, aiQueue, reindexProgress, order } = makeService();
+
+    await service.reindex(WORKSPACE_ID);
+
+    // The pre-seed carries the real page count AND a SHORT ttl (3rd arg) so a
+    // de-duplicated enqueue against a just-finishing job can't leave a phantom
+    // "reindexing: 0 of N" stuck for the full record TTL (F10).
+    expect(reindexProgress.start).toHaveBeenCalledWith(
+      WORKSPACE_ID,
+      478,
+      expect.any(Number),
+    );
+    const ttl = reindexProgress.start.mock.calls[0][2];
+    // Short pre-seed TTL, distinct from the full 1h (3600s) record TTL, but
+    // pinned to the client poll cap (120s) so a still-pending run can't expire
+    // into a false "done" while the client is still polling (F11).
+    expect(ttl).toBe(120);
+    expect(aiQueue.add).toHaveBeenCalledTimes(1);
+    // Seed must precede the enqueue so the first poll already reports done=0.
+    expect(order).toEqual(['start', 'add']);
+  });
+
+  it('does NOT re-seed when a run is already active (mid-run re-trigger)', async () => {
+    const { service, aiQueue, reindexProgress } = makeService();
+    // An active record exists -> a second click must not reset the counter.
+    reindexProgress.get.mockResolvedValue({
+      total: 478,
+      done: 120,
+      startedAt: Date.now(),
+    });
+
+    await service.reindex(WORKSPACE_ID);
+
+    expect(reindexProgress.start).not.toHaveBeenCalled();
+    // The enqueue still runs (and de-duplicates against the active job).
+    expect(aiQueue.add).toHaveBeenCalledTimes(1);
+  });
+
+  it('clears the seed it just wrote and re-throws when enqueue fails', async () => {
+    const { service, aiQueue, reindexProgress } = makeService();
+    // This call seeds (get() is null) but the enqueue then blows up
+    // (Redis hiccup/shutdown) -> the worker never runs and never clear()s, so
+    // reindex() must roll back its own seed to avoid a 1h stuck "reindexing".
+    const boom = new Error('redis down');
+    aiQueue.add.mockRejectedValue(boom);
+
+    await expect(service.reindex(WORKSPACE_ID)).rejects.toBe(boom);
+
+    expect(reindexProgress.start).toHaveBeenCalledWith(
+      WORKSPACE_ID,
+      478,
+      expect.any(Number),
+    );
+    expect(reindexProgress.clear).toHaveBeenCalledWith(WORKSPACE_ID);
+  });
+
+  it('does NOT clear a concurrent active run when enqueue fails (no seed)', async () => {
+    const { service, aiQueue, reindexProgress } = makeService();
+    // A run is already active, so THIS call does not seed; if the enqueue then
+    // fails it must NOT wipe the live worker's record.
+    reindexProgress.get.mockResolvedValue({
+      total: 478,
+      done: 120,
+      startedAt: Date.now(),
+    });
+    const boom = new Error('redis down');
+    aiQueue.add.mockRejectedValue(boom);
+
+    await expect(service.reindex(WORKSPACE_ID)).rejects.toBe(boom);
+
+    expect(reindexProgress.start).not.toHaveBeenCalled();
+    expect(reindexProgress.clear).not.toHaveBeenCalled();
+  });
+});

apps/server/src/integrations/ai/ai-settings.service.ts

@@ -8,6 +8,7 @@ import { AiProviderCredentialsRepo } from '@docmost/db/repos/ai-chat/ai-provider
 import { PageEmbeddingRepo } from '@docmost/db/repos/ai-chat/page-embedding.repo';
 import { PageRepo } from '@docmost/db/repos/page/page.repo';
 import { SecretBoxService } from '../crypto/secret-box';
+import { EmbeddingReindexProgressService } from './embedding-reindex-progress.service';
 import {
   AiDriver,
   AiProviderSettings,
@@ -30,6 +31,30 @@ export function parsePositiveInt(raw: unknown): number | undefined {
   return Number.isFinite(n) && n > 0 ? Math.floor(n) : undefined;
 }
 
+/**
+ * TTL (seconds) for the enqueue-time progress PRE-SEED written by `reindex()`
+ * before the worker starts. Deliberately SHORT relative to the full 1h record
+ * TTL: if `aiQueue.add()` de-duplicates against a job that is just finishing
+ * (the worker's finally already ran `clear()` but removeOnComplete hasn't yet
+ * removed the job), no new worker runs to overwrite/clear this seed — so this
+ * shorter TTL lets the phantom "reindexing: 0 of N" expire instead of sticking
+ * for the full 1h record TTL. A worker that DOES start re-seeds with the full
+ * TTL, so a real run is unaffected.
+ *
+ * It MUST be >= the client poll cap (REINDEX_POLL_CAP_MS = 120000ms in
+ * ai-provider-settings.tsx) though: the AI_QUEUE worker runs at concurrency 1
+ * and shares the queue with page-level embedding jobs, so a queued reindex can
+ * wait well beyond a few dozen seconds before the worker re-seeds with the full
+ * TTL. If the pre-seed expired while the job is still pending, `get()` returns
+ * null and getMasked() falls back to the steady-state COUNT (indexedPages ==
+ * totalPages, reindexing=false) — the client reads that as "done & fully
+ * indexed", clears its deadline and STOPS polling, so the admin never sees the
+ * real climb. Pinning the pre-seed TTL to the client cap means a deduped phantom
+ * is bounded to ~120s — the same window the client already polls — and a genuine
+ * pending run never expires-into-"done" inside that window.
+ */
+const PRE_SEED_TTL_SECONDS = 120;
+
 /**
  * Shape of the partial update accepted by `update`. Mirrors the validated
  * controller DTO. `apiKey` / `embeddingApiKey` are write-only: undefined =
@@ -74,6 +99,7 @@ export class AiSettingsService {
     private readonly pageEmbeddingRepo: PageEmbeddingRepo,
     private readonly pageRepo: PageRepo,
     private readonly secretBox: SecretBoxService,
+    private readonly reindexProgress: EmbeddingReindexProgressService,
     @InjectQueue(QueueName.AI_QUEUE) private readonly aiQueue: Queue,
   ) {}
 
@@ -100,21 +126,63 @@ export class AiSettingsService {
       .remove(`ai-search-disabled-${workspaceId}`)
       .catch(() => undefined);
 
+    // Seed a live progress record BEFORE enqueueing so the very first status
+    // poll already reports done=0 (the reindex POST returns the PRE-job counts,
+    // so without this seed the first poll would still show "total of total").
+    // `totalPages` uses countEmbeddablePages — the SAME set the worker iterates
+    // and the SAME denominator the status endpoint reports, so the live and
+    // steady-state totals match.
+    //
+    // ONLY seed when no run is active: aiQueue.add() de-duplicates an already-
+    // running reindex, so a mid-run re-trigger (second click / second admin /
+    // second tab) must NOT reset the visible counter to 0 — that would
+    // understate the live worker's real position for the rest of the run. The
+    // worker's own start() at run begin is the single authoritative reset.
+    let seeded = false;
+    if ((await this.reindexProgress.get(workspaceId)) === null) {
+      const totalPages = await this.pageRepo.countEmbeddablePages(workspaceId);
+      // Short TTL (vs the full 1h record TTL): if add() below de-duplicates
+      // against a just-finishing job whose worker already clear()ed but isn't
+      // removed yet, no worker runs to clear this seed — the shorter TTL expires
+      // the phantom record rather than leaving a stuck "reindexing: 0 of N" for
+      // the full record TTL. It is kept >= the client poll cap (120s) so a
+      // genuine but still-pending run never expires into a false "done" while
+      // the client is still polling (see PRE_SEED_TTL_SECONDS).
+      await this.reindexProgress.start(
+        workspaceId,
+        totalPages,
+        PRE_SEED_TTL_SECONDS,
+      );
+      seeded = true;
+    }
+
     const jobId = `ai-reindex-${workspaceId}`;
     // Clear a prior non-active entry so a stale job can't block this reindex.
     // A locked/active job is left in place (remove() no-ops) and the add() below
     // de-duplicates against it, keeping the in-progress pass.
     await this.aiQueue.remove(jobId).catch(() => undefined);
 
-    await this.aiQueue.add(
-      QueueJob.WORKSPACE_CREATE_EMBEDDINGS,
-      { workspaceId },
-      {
-        jobId,
-        removeOnComplete: true,
-        removeOnFail: true,
-      },
-    );
+    try {
+      await this.aiQueue.add(
+        QueueJob.WORKSPACE_CREATE_EMBEDDINGS,
+        { workspaceId },
+        {
+          jobId,
+          removeOnComplete: true,
+          removeOnFail: true,
+        },
+      );
+    } catch (err) {
+      // If the enqueue fails (Redis hiccup/shutdown) the worker never runs, so
+      // its finally->clear() never fires. Roll back the seed WE just wrote so
+      // the status endpoint doesn't report a stuck "reindexing: 0 of N" for the
+      // full TTL. Only clear when this call did the seed — never wipe a
+      // concurrent active run's record (get() was non-null, seeded=false).
+      if (seeded) {
+        await this.reindexProgress.clear(workspaceId);
+      }
+      throw err;
+    }
   }
 
   /**
@@ -253,13 +321,33 @@ export class AiSettingsService {
       hasSttApiKey = !!creds?.sttApiKeyEnc;
     }
 
-    // totalPages now counts only pages with embeddable content (non-empty text
-    // or already-stored embeddings), so empty/text-less pages don't keep the
-    // "Indexed N of M pages" bar below 100% forever.
-    const [indexedPages, totalPages] = await Promise.all([
-      this.pageEmbeddingRepo.countIndexedPages(workspaceId),
-      this.pageRepo.countEmbeddablePages(workspaceId),
-    ]);
+    // While a reindex run is active, report its LIVE progress (done climbs 0 ->
+    // total) so the settings UI can watch it advance. Read progress FIRST and
+    // short-circuit: this endpoint is polled every ~5s for the whole run, so when
+    // a record is active we skip the two coverage COUNTs entirely (their results
+    // would be discarded anyway). Without the live progress the counter never
+    // drops: the per-page reindex hard-replaces rows in its own small
+    // transaction, so countIndexedPages stays ~= total for the whole run. With no
+    // active record we fall back to the steady-state DB coverage count, which
+    // preserves the existing display and the client's "done == total -> stop
+    // polling" condition (the run ends -> record cleared -> DB count == total).
+    //
+    // The fallback `totalPages` counts only pages with embeddable content
+    // (non-empty text, content-borne text, or already-stored embeddings), so
+    // empty/text-less pages don't keep the "Indexed N of M pages" bar below 100%
+    // forever.
+    const progress = await this.reindexProgress.get(workspaceId);
+    let indexedPages: number;
+    let totalPages: number;
+    if (progress) {
+      indexedPages = progress.done;
+      totalPages = progress.total;
+    } else {
+      [indexedPages, totalPages] = await Promise.all([
+        this.pageEmbeddingRepo.countIndexedPages(workspaceId),
+        this.pageRepo.countEmbeddablePages(workspaceId),
+      ]);
+    }
 
     return {
       driver: provider.driver,
@@ -281,6 +369,8 @@ export class AiSettingsService {
       hasSttApiKey,
       indexedPages,
       totalPages,
+      // Optional hint for the client: a reindex run is currently in progress.
+      reindexing: progress != null,
     };
   }
 

apps/server/src/integrations/ai/ai.module.ts

@@ -5,6 +5,7 @@ import { QueueName } from '../queue/constants';
 import { AiService } from './ai.service';
 import { AiSettingsService } from './ai-settings.service';
 import { AiSettingsController } from './ai-settings.controller';
+import { EmbeddingReindexProgressService } from './embedding-reindex-progress.service';
 
 /**
  * LLM driver + provider-settings unit (§6.2/§6.4).
@@ -19,7 +20,7 @@ import { AiSettingsController } from './ai-settings.controller';
     BullModule.registerQueue({ name: QueueName.AI_QUEUE }),
   ],
   controllers: [AiSettingsController],
-  providers: [AiService, AiSettingsService],
-  exports: [AiService, AiSettingsService],
+  providers: [AiService, AiSettingsService, EmbeddingReindexProgressService],
+  exports: [AiService, AiSettingsService, EmbeddingReindexProgressService],
 })
 export class AiModule {}

apps/server/src/integrations/ai/ai.types.ts

@@ -146,4 +146,7 @@ export interface MaskedAiSettings {
   // RAG indexing coverage for the settings UI.
   indexedPages: number;
   totalPages: number;
+  // True while a full workspace reindex is actively running (the counts above
+  // then reflect the live run progress rather than the steady-state DB count).
+  reindexing?: boolean;
 }

apps/server/src/integrations/ai/embedding-reindex-progress.service.spec.ts

@@ -0,0 +1,179 @@
+import { EmbeddingReindexProgressService } from './embedding-reindex-progress.service';
+import type { RedisService } from '@nestjs-labs/nestjs-ioredis';
+import type { Redis } from 'ioredis';
+
+/**
+ * Unit tests for the Redis-backed reindex-progress store.
+ *
+ * The store is a thin, BEST-EFFORT wrapper: writes (start/increment) issue an
+ * hset/hincrby + expire pipeline and must SWALLOW Redis errors (progress is
+ * cosmetic — it must never break a reindex); reads (get) must map a valid hash
+ * to a ReindexProgress and degrade to null on a malformed/missing record or a
+ * Redis failure. We drive it with a hand-rolled fake ioredis (the project mocks
+ * Redis with plain fakes, see public-share limiter specs).
+ */
+describe('EmbeddingReindexProgressService', () => {
+  const WORKSPACE_ID = 'ws-1';
+  const KEY = 'ai:reindex:progress:ws-1';
+
+  /**
+   * Build a fake ioredis whose `multi()` returns a chainable recorder and whose
+   * `hgetall`/`del` are configurable jest mocks. `execImpl` lets a test make the
+   * pipeline reject (to assert error-swallowing).
+   */
+  function makeRedis(opts: { execImpl?: () => Promise<unknown> } = {}) {
+    const exec = jest
+      .fn()
+      .mockImplementation(opts.execImpl ?? (() => Promise.resolve([])));
+    // mockReturnThis() returns the call's `this` (the multi object), so the
+    // chain hset().expire().exec() resolves correctly.
+    const multiObj = {
+      hset: jest.fn().mockReturnThis(),
+      hincrby: jest.fn().mockReturnThis(),
+      expire: jest.fn().mockReturnThis(),
+      exec,
+    };
+    const multi = jest.fn(() => multiObj);
+    const hgetall = jest.fn().mockResolvedValue({});
+    const del = jest.fn().mockResolvedValue(1);
+    const redis = { multi, hgetall, del } as unknown as Redis;
+    return { redis, multiObj, multi, hgetall, del, exec };
+  }
+
+  function makeService(redis: Redis) {
+    const redisService = {
+      getOrThrow: () => redis,
+    } as unknown as RedisService;
+    return new EmbeddingReindexProgressService(redisService);
+  }
+
+  describe('get', () => {
+    it('maps a valid hash to a ReindexProgress object', async () => {
+      const { redis, hgetall } = makeRedis();
+      hgetall.mockResolvedValue({ total: '478', done: '120', startedAt: '1000' });
+      const service = makeService(redis);
+
+      await expect(service.get(WORKSPACE_ID)).resolves.toEqual({
+        total: 478,
+        done: 120,
+        startedAt: 1000,
+      });
+      expect(hgetall).toHaveBeenCalledWith(KEY);
+    });
+
+    it('returns null for an empty hash (no record)', async () => {
+      const { redis, hgetall } = makeRedis();
+      hgetall.mockResolvedValue({});
+      await expect(makeService(redis).get(WORKSPACE_ID)).resolves.toBeNull();
+    });
+
+    it('returns null when `total` is missing (partial record)', async () => {
+      const { redis, hgetall } = makeRedis();
+      hgetall.mockResolvedValue({ done: '5' });
+      await expect(makeService(redis).get(WORKSPACE_ID)).resolves.toBeNull();
+    });
+
+    it('returns null for a non-numeric total', async () => {
+      const { redis, hgetall } = makeRedis();
+      hgetall.mockResolvedValue({ total: 'abc', done: '1', startedAt: '1' });
+      await expect(makeService(redis).get(WORKSPACE_ID)).resolves.toBeNull();
+    });
+
+    it('returns null for a non-numeric done', async () => {
+      const { redis, hgetall } = makeRedis();
+      hgetall.mockResolvedValue({ total: '10', done: 'xyz', startedAt: '1' });
+      await expect(makeService(redis).get(WORKSPACE_ID)).resolves.toBeNull();
+    });
+
+    it('coerces a non-finite startedAt to 0', async () => {
+      const { redis, hgetall } = makeRedis();
+      hgetall.mockResolvedValue({ total: '10', done: '2', startedAt: 'nope' });
+      await expect(makeService(redis).get(WORKSPACE_ID)).resolves.toEqual({
+        total: 10,
+        done: 2,
+        startedAt: 0,
+      });
+    });
+
+    it('degrades to null when hgetall throws (degradation contract)', async () => {
+      const { redis, hgetall } = makeRedis();
+      hgetall.mockRejectedValue(new Error('redis down'));
+      await expect(makeService(redis).get(WORKSPACE_ID)).resolves.toBeNull();
+    });
+  });
+
+  describe('start', () => {
+    it('issues hset + expire on the workspace key', async () => {
+      const { redis, multiObj } = makeRedis();
+      await makeService(redis).start(WORKSPACE_ID, 478);
+
+      expect(multiObj.hset).toHaveBeenCalledWith(
+        KEY,
+        expect.objectContaining({ total: '478', done: '0' }),
+      );
+      expect(multiObj.expire).toHaveBeenCalledWith(KEY, expect.any(Number));
+      expect(multiObj.exec).toHaveBeenCalledTimes(1);
+    });
+
+    it('defaults the expire TTL to the full 1h record TTL', async () => {
+      const { redis, multiObj } = makeRedis();
+      await makeService(redis).start(WORKSPACE_ID, 478);
+      // Default ttl = full record TTL (60 * 60) so a real run never expires
+      // mid-flight before the worker refreshes it on each increment.
+      expect(multiObj.expire).toHaveBeenCalledWith(KEY, 60 * 60);
+    });
+
+    it('honours an explicit short ttlSeconds for the enqueue-time pre-seed (F10)', async () => {
+      const { redis, multiObj } = makeRedis();
+      // The reindex() pre-seed passes a short ttl so a phantom record left by a
+      // de-duplicated enqueue expires in seconds, not after the full 1h TTL.
+      await makeService(redis).start(WORKSPACE_ID, 478, 45);
+      expect(multiObj.expire).toHaveBeenCalledWith(KEY, 45);
+    });
+
+    it('swallows a thrown Redis error (best-effort)', async () => {
+      const { redis } = makeRedis({
+        execImpl: () => Promise.reject(new Error('redis down')),
+      });
+      await expect(
+        makeService(redis).start(WORKSPACE_ID, 1),
+      ).resolves.toBeUndefined();
+    });
+  });
+
+  describe('increment', () => {
+    it('issues hincrby + expire on the workspace key', async () => {
+      const { redis, multiObj } = makeRedis();
+      await makeService(redis).increment(WORKSPACE_ID);
+
+      expect(multiObj.hincrby).toHaveBeenCalledWith(KEY, 'done', 1);
+      expect(multiObj.expire).toHaveBeenCalledWith(KEY, expect.any(Number));
+      expect(multiObj.exec).toHaveBeenCalledTimes(1);
+    });
+
+    it('swallows a thrown Redis error (best-effort)', async () => {
+      const { redis } = makeRedis({
+        execImpl: () => Promise.reject(new Error('redis down')),
+      });
+      await expect(
+        makeService(redis).increment(WORKSPACE_ID),
+      ).resolves.toBeUndefined();
+    });
+  });
+
+  describe('clear', () => {
+    it('deletes the workspace key', async () => {
+      const { redis, del } = makeRedis();
+      await makeService(redis).clear(WORKSPACE_ID);
+      expect(del).toHaveBeenCalledWith(KEY);
+    });
+
+    it('swallows a thrown Redis error (best-effort)', async () => {
+      const { redis, del } = makeRedis();
+      del.mockRejectedValue(new Error('redis down'));
+      await expect(
+        makeService(redis).clear(WORKSPACE_ID),
+      ).resolves.toBeUndefined();
+    });
+  });
+});

apps/server/src/integrations/ai/embedding-reindex-progress.service.ts

@@ -0,0 +1,162 @@
+import { Injectable, Logger } from '@nestjs/common';
+import { RedisService } from '@nestjs-labs/nestjs-ioredis';
+import type { Redis } from 'ioredis';
+
+/**
+ * Live progress of an in-flight workspace embeddings reindex run.
+ * `total` is the number of pages the run will process, `done` how many it has
+ * already processed (success OR handled failure), `startedAt` the epoch-ms the
+ * record was created.
+ */
+export interface ReindexProgress {
+  total: number;
+  done: number;
+  startedAt: number;
+}
+
+/** Redis key namespace for the per-workspace reindex-progress record. */
+const KEY_PREFIX = 'ai:reindex:progress:';
+
+/**
+ * TTL (seconds) on the progress record so a crashed/aborted worker that never
+ * reaches its `clear()` finally can still self-clean instead of leaving a stuck
+ * "reindexing" state. Refreshed on every increment so a long run never expires
+ * mid-flight; on a crash it disappears within TTL of the last processed page.
+ *
+ * INTENTIONALLY tied to WRITE progress (start/increment) only — never refreshed
+ * on get(). Refreshing on read would keep a dead worker's record alive forever
+ * as long as a client keeps polling (a permanently stuck reindexing:true). The
+ * clear() in the worker's finally handles normal completion; a dead worker's
+ * record expires after TTL, and the client's own poll cap stops polling anyway.
+ */
+const TTL_SECONDS = 60 * 60; // 1h
+
+/**
+ * Cluster-wide store for the live progress of a workspace embeddings reindex.
+ *
+ * The reindex runs in a BullMQ worker (AI_QUEUE) that may be a DIFFERENT process
+ * than the API handling the settings-status GET, so the progress must live in
+ * the shared Redis — we reuse the same global ioredis client (RedisService from
+ * @nestjs-labs/nestjs-ioredis) that backs BullMQ and the other anti-abuse
+ * limiters, adding NO new Redis config.
+ *
+ * Everything here is best-effort and COSMETIC: progress only drives the "Indexed
+ * X of Y" counter while a reindex is running. Any Redis failure degrades to the
+ * existing steady-state behaviour (the status falls back to the DB coverage
+ * count), so reads fail to `null` and writes are swallowed — a reindex must
+ * never break because progress reporting did.
+ *
+ * Stored as a Redis HASH so `done` can be bumped with an atomic HINCRBY (the
+ * worker is the only writer of `done`, but HINCRBY also keeps us off a
+ * read-modify-write race and preserves the other fields).
+ */
+@Injectable()
+export class EmbeddingReindexProgressService {
+  private readonly logger = new Logger(EmbeddingReindexProgressService.name);
+  private readonly redis: Redis;
+
+  constructor(redisService: RedisService) {
+    this.redis = redisService.getOrThrow();
+  }
+
+  private key(workspaceId: string): string {
+    return KEY_PREFIX + workspaceId;
+  }
+
+  /**
+   * Begin (or reset) the progress record for a workspace: `total` pages, `done`
+   * back to 0, `startedAt` now. Called twice for a run, BOTH with the real page
+   * count (countEmbeddablePages) so the two totals coincide: once at reindex
+   * enqueue time (so the very first status poll already reports done=0) and again
+   * at the worker start (which re-asserts the same total and resets `done`).
+   * Resets `done` to 0 so a re-trigger never inherits a stale count.
+   *
+   * `ttlSeconds` lets the caller pick the record's lifetime. The enqueue-time
+   * pre-seed passes a SHORT ttl: if `aiQueue.add()` de-duplicates against a job
+   * that is just finishing (its worker hasn't yet removed the job but already
+   * ran its `clear()`), no new worker starts to clear this phantom seed, so a
+   * short ttl lets it expire in seconds instead of sticking for the full TTL.
+   * The worker's own `start()` at the begin of a real run overwrites this entry
+   * and raises the ttl back to the default full TTL.
+   */
+  async start(
+    workspaceId: string,
+    total: number,
+    ttlSeconds: number = TTL_SECONDS,
+  ): Promise<void> {
+    const key = this.key(workspaceId);
+    try {
+      await this.redis
+        .multi()
+        .hset(key, {
+          total: String(total),
+          done: '0',
+          startedAt: String(Date.now()),
+        })
+        .expire(key, ttlSeconds)
+        .exec();
+    } catch (err) {
+      this.logger.warn(
+        `reindex-progress start failed for workspace ${workspaceId}; ` +
+          `progress reporting disabled for this run: ${(err as Error).message}`,
+      );
+    }
+  }
+
+  /**
+   * Bump the processed-page counter by one and refresh the TTL. Atomic and
+   * best-effort: a missing key (cleared/expired) would be recreated with only
+   * `done`, but `get()` treats a record without a numeric `total` as inactive,
+   * so that partial state safely reads as "no active reindex".
+   */
+  async increment(workspaceId: string): Promise<void> {
+    const key = this.key(workspaceId);
+    try {
+      await this.redis.multi().hincrby(key, 'done', 1).expire(key, TTL_SECONDS).exec();
+    } catch (err) {
+      this.logger.warn(
+        `reindex-progress increment failed for workspace ${workspaceId}: ` +
+          `${(err as Error).message}`,
+      );
+    }
+  }
+
+  /**
+   * Remove the progress record. Called in the worker's `finally` so a completed,
+   * aborted, or unconfigured-early-return run never leaves a stuck record; the
+   * status then falls back to the DB coverage count.
+   */
+  async clear(workspaceId: string): Promise<void> {
+    try {
+      await this.redis.del(this.key(workspaceId));
+    } catch (err) {
+      this.logger.warn(
+        `reindex-progress clear failed for workspace ${workspaceId} ` +
+          `(self-cleans via TTL): ${(err as Error).message}`,
+      );
+    }
+  }
+
+  /**
+   * Read the live progress, or `null` when no reindex is active (no record, an
+   * expired record, or a partial record without a numeric `total`). On a Redis
+   * error returns `null` so the status endpoint degrades to its DB count.
+   */
+  async get(workspaceId: string): Promise<ReindexProgress | null> {
+    try {
+      const data = await this.redis.hgetall(this.key(workspaceId));
+      if (!data || data.total === undefined) return null;
+      const total = Number(data.total);
+      const done = Number(data.done);
+      const startedAt = Number(data.startedAt);
+      if (!Number.isFinite(total) || !Number.isFinite(done)) return null;
+      return { total, done, startedAt: Number.isFinite(startedAt) ? startedAt : 0 };
+    } catch (err) {
+      this.logger.warn(
+        `reindex-progress read failed for workspace ${workspaceId}; ` +
+          `falling back to DB count: ${(err as Error).message}`,
+      );
+      return null;
+    }
+  }
+}

apps/server/src/integrations/storage/storage.service.spec.ts

@@ -1,18 +1,110 @@
+import { Readable } from 'stream';
 import { StorageService } from './storage.service';
+import type { StorageDriver } from './interfaces';
 
-// Direct instantiation with a stub driver. The Test.createTestingModule form
-// failed to resolve the STORAGE_DRIVER_TOKEN at compile(); this smoke test only
-// needs the service to construct.
-describe('StorageService', () => {
+/**
+ * StorageService is a thin facade over the injected StorageDriver: each public
+ * method must forward to the driver with the SAME arguments and return/await the
+ * driver's result unchanged (the read paths return it; the write paths await it).
+ * A mock driver lets us assert that delegation exactly, with no real S3/disk IO.
+ */
+describe('StorageService delegation', () => {
+  // Every driver method is a jest mock so we can assert call args + return passing.
+  function buildDriver(): jest.Mocked<StorageDriver> {
+    return {
+      upload: jest.fn().mockResolvedValue(undefined),
+      uploadStream: jest.fn().mockResolvedValue(undefined),
+      copy: jest.fn().mockResolvedValue(undefined),
+      read: jest.fn(),
+      readStream: jest.fn(),
+      readRangeStream: jest.fn(),
+      exists: jest.fn(),
+      getUrl: jest.fn(),
+      getSignedUrl: jest.fn(),
+      delete: jest.fn().mockResolvedValue(undefined),
+      getDriver: jest.fn(),
+      getDriverName: jest.fn(),
+      getConfig: jest.fn(),
+    } as unknown as jest.Mocked<StorageDriver>;
+  }
+
+  let driver: jest.Mocked<StorageDriver>;
   let service: StorageService;
 
   beforeEach(() => {
-    service = new StorageService(
-      {} as any, // storageDriver
-    );
+    driver = buildDriver();
+    service = new StorageService(driver as unknown as StorageDriver);
   });
 
-  it('should be defined', () => {
-    expect(service).toBeDefined();
+  it('upload forwards path + content to the driver', async () => {
+    const buf = Buffer.from('data');
+    await service.upload('a/b.png', buf);
+    expect(driver.upload).toHaveBeenCalledWith('a/b.png', buf);
+  });
+
+  it('uploadStream forwards path, stream and options', async () => {
+    const stream = Readable.from(['x']);
+    await service.uploadStream('a/b.bin', stream, { recreateClient: true });
+    expect(driver.uploadStream).toHaveBeenCalledWith('a/b.bin', stream, {
+      recreateClient: true,
+    });
+  });
+
+  it('copy forwards both paths', async () => {
+    await service.copy('from.txt', 'to.txt');
+    expect(driver.copy).toHaveBeenCalledWith('from.txt', 'to.txt');
+  });
+
+  it('read returns the driver buffer unchanged', async () => {
+    const buf = Buffer.from('content');
+    driver.read.mockResolvedValue(buf);
+    await expect(service.read('f.txt')).resolves.toBe(buf);
+    expect(driver.read).toHaveBeenCalledWith('f.txt');
+  });
+
+  it('readStream returns the driver stream unchanged', async () => {
+    const stream = Readable.from(['y']);
+    driver.readStream.mockResolvedValue(stream);
+    await expect(service.readStream('f.bin')).resolves.toBe(stream);
+    expect(driver.readStream).toHaveBeenCalledWith('f.bin');
+  });
+
+  it('readRangeStream forwards the range object and returns the stream', async () => {
+    const stream = Readable.from(['z']);
+    driver.readRangeStream.mockResolvedValue(stream);
+    const range = { start: 0, end: 99 };
+    await expect(service.readRangeStream('f.bin', range)).resolves.toBe(stream);
+    expect(driver.readRangeStream).toHaveBeenCalledWith('f.bin', range);
+  });
+
+  it('exists returns the driver boolean', async () => {
+    driver.exists.mockResolvedValue(false);
+    await expect(service.exists('missing')).resolves.toBe(false);
+    expect(driver.exists).toHaveBeenCalledWith('missing');
+  });
+
+  it('getSignedUrl forwards path + expiry and returns the signed url', async () => {
+    driver.getSignedUrl.mockResolvedValue('https://signed/url');
+    await expect(service.getSignedUrl('f.png', 600)).resolves.toBe(
+      'https://signed/url',
+    );
+    expect(driver.getSignedUrl).toHaveBeenCalledWith('f.png', 600);
+  });
+
+  it('getUrl returns the driver url synchronously', () => {
+    driver.getUrl.mockReturnValue('https://cdn/f.png');
+    expect(service.getUrl('f.png')).toBe('https://cdn/f.png');
+    expect(driver.getUrl).toHaveBeenCalledWith('f.png');
+  });
+
+  it('delete forwards the path', async () => {
+    await service.delete('old.txt');
+    expect(driver.delete).toHaveBeenCalledWith('old.txt');
+  });
+
+  it('getDriverName returns the driver name', () => {
+    driver.getDriverName.mockReturnValue('s3');
+    expect(service.getDriverName()).toBe('s3');
+    expect(driver.getDriverName).toHaveBeenCalledTimes(1);
   });
 });

apps/server/src/ws/adapter/ws-redis.adapter.ts

@@ -1,3 +1,4 @@
+import { Logger } from '@nestjs/common';
 import { IoAdapter } from '@nestjs/platform-socket.io';
 import { ServerOptions } from 'socket.io';
 import { createAdapter } from '@socket.io/redis-adapter';
@@ -9,8 +10,11 @@ import {
 } from '../../common/helpers';
 
 export class WsRedisIoAdapter extends IoAdapter {
+  private readonly logger = new Logger(WsRedisIoAdapter.name);
   private adapterConstructor: ReturnType<typeof createAdapter>;
   private redisConfig: RedisConfig;
+  private pubClient: Redis;
+  private subClient: Redis;
 
   async connectToRedis(): Promise<void> {
     this.redisConfig = parseRedisUrl(process.env.REDIS_URL);
@@ -23,8 +27,13 @@ export class WsRedisIoAdapter extends IoAdapter {
     const pubClient = new Redis(process.env.REDIS_URL, options);
     const subClient = new Redis(process.env.REDIS_URL, options);
 
-    pubClient.on('error', (err) => () => {});
-    subClient.on('error', (err) => () => {});
+    pubClient.on('error', (err) => this.logger.error('socket.io redis pub client error', err));
+    subClient.on('error', (err) => this.logger.error('socket.io redis sub client error', err));
+
+    // Hold references so the pub/sub connections can be torn down on shutdown
+    // (see dispose()); otherwise these ioredis sockets leak as active handles.
+    this.pubClient = pubClient;
+    this.subClient = subClient;
 
     this.adapterConstructor = createAdapter(pubClient, subClient);
   }
@@ -34,4 +43,26 @@ export class WsRedisIoAdapter extends IoAdapter {
     server.adapter(this.adapterConstructor);
     return server;
   }
+
+  /**
+   * Called once by Nest's SocketModule during application shutdown, after every
+   * socket.io server has been closed. The @socket.io/redis-adapter never owns
+   * the lifecycle of the ioredis pub/sub clients it is handed, so we close them
+   * here to avoid leaking their TCP handles on shutdown (see issue #255).
+   *
+   * Uses disconnect(false) to mirror the sibling pub/sub pair in
+   * collaboration/extensions/redis-sync (redis-sync.extension.ts onDestroy):
+   * an immediate close with no graceful QUIT round-trip and no auto-reconnect,
+   * which is what we want for idle adapter clients during teardown.
+   */
+  async dispose(): Promise<void> {
+    await super.dispose();
+
+    // dispose() is invoked once per shutdown; null the refs so a second call
+    // (or any post-shutdown path) cannot act on already-closed clients.
+    this.pubClient?.disconnect(false);
+    this.subClient?.disconnect(false);
+    this.pubClient = undefined;
+    this.subClient = undefined;
+  }
 }

apps/server/test/integration/page-embeddable-ids-lockstep.int-spec.ts

@@ -0,0 +1,160 @@
+import { Kysely } from 'kysely';
+import { randomUUID } from 'node:crypto';
+import { PageRepo } from '@docmost/db/repos/page/page.repo';
+import { SpaceMemberRepo } from '@docmost/db/repos/space/space-member.repo';
+import { EventEmitter2 } from '@nestjs/event-emitter';
+import { getTestDb, destroyTestDb, createWorkspace, createSpace } from './db';
+
+/**
+ * `PageRepo.getEmbeddablePageIds` MUST stay in lockstep with
+ * `PageRepo.countEmbeddablePages` (page.repo.ts) — the bulk reindex iterates the
+ * ID set while the status endpoint reports the count as the live denominator, so
+ * if the two predicates ever diverge the "done X of Y" counter ends on the wrong
+ * total. Both share the SAME WHERE: a page qualifies iff it is non-deleted AND
+ * (text_content has a non-whitespace char OR — when text_content is empty — its
+ * content JSON has a text node OR it has a non-deleted embedding row).
+ *
+ * This is a DB-level invariant: the predicate lives in raw SQL (`text_content ~
+ * '[^[:space:]]'`, `content::text ~ '"type"[[:space:]]*:[[:space:]]*"text"'`) and an EXISTS subquery, so a unit test with mocked Kysely
+ * cannot observe it. We seed every boundary case against real Postgres and
+ * assert the returned ID set EQUALS the count (and is exactly the expected set).
+ * A future edit that touches one predicate but not the other turns this red.
+ */
+describe('PageRepo embeddable-page set: getEmbeddablePageIds <-> countEmbeddablePages [integration]', () => {
+  let db: Kysely<any>;
+  let repo: PageRepo;
+  let workspaceId: string;
+  let spaceId: string;
+
+  beforeAll(async () => {
+    db = getTestDb();
+    // Only the Kysely-backed query methods under test are exercised, so the
+    // SpaceMemberRepo / EventEmitter2 deps are never touched — stub them.
+    repo = new PageRepo(
+      db as any,
+      {} as unknown as SpaceMemberRepo,
+      {} as unknown as EventEmitter2,
+    );
+    workspaceId = (await createWorkspace(db)).id;
+    spaceId = (await createSpace(db, workspaceId)).id;
+  });
+
+  afterAll(async () => {
+    await destroyTestDb();
+  });
+
+  // Insert a page with explicit text_content / content / deleted_at (createPage
+  // in db.ts sets none), returning its id so the test can assert membership.
+  // `content` is the ProseMirror doc JSON (jsonb): postgres.js serializes a plain
+  // object to JSON for jsonb columns, so we pass it through only when supplied so
+  // the rest of the rows keep the DB default.
+  async function insertPage(args: {
+    textContent: string | null;
+    content?: unknown;
+    deletedAt?: Date | null;
+  }): Promise<string> {
+    const id = randomUUID();
+    await db
+      .insertInto('pages')
+      .values({
+        id,
+        slugId: `slug-${id.slice(0, 8)}`,
+        title: `page-${id.slice(0, 8)}`,
+        spaceId,
+        workspaceId,
+        textContent: args.textContent,
+        ...(args.content !== undefined ? { content: args.content as any } : {}),
+        deletedAt: args.deletedAt ?? null,
+      })
+      .execute();
+    return id;
+  }
+
+  // Insert one embedding chunk row for a page (NOT NULL columns + deleted_at).
+  async function insertEmbedding(
+    pageId: string,
+    opts: { deletedAt?: Date | null } = {},
+  ): Promise<void> {
+    await db
+      .insertInto('pageEmbeddings')
+      .values({
+        id: randomUUID(),
+        workspaceId,
+        pageId,
+        spaceId,
+        chunkIndex: 0,
+        chunkStart: 0,
+        chunkLength: 1,
+        content: 'x',
+        modelName: 'test-model',
+        modelDimensions: 1,
+        deletedAt: opts.deletedAt ?? null,
+      })
+      .execute();
+  }
+
+  it('returns exactly the embeddable set and its size equals countEmbeddablePages', async () => {
+    // IN the set --------------------------------------------------------------
+    // (a) non-deleted page with real body text.
+    const withText = await insertPage({ textContent: 'hello world' });
+    // (b) non-deleted page with NO text but a live embedding row (EXISTS clause:
+    //     a page that lost its text yet still has stale vectors must be visited
+    //     so the reindex can clear them).
+    const noTextLiveEmbedding = await insertPage({ textContent: null });
+    await insertEmbedding(noTextLiveEmbedding);
+    // (c) non-deleted page with EMPTY text_content but ProseMirror `content` JSON
+    //     carrying a real text node — the content-JSON clause. This pins BOTH the
+    //     third OR-clause AND the space-after-colon: jsonb stores the key/value
+    //     separator as `"type": "text"` (a space after the colon), which is why
+    //     the predicate needs `[[:space:]]*`. `reindexPage` extracts this text, so
+    //     the page IS embeddable and the reindex MUST visit it.
+    const noTextContentDoc = await insertPage({
+      textContent: null,
+      content: {
+        type: 'doc',
+        content: [
+          { type: 'paragraph', content: [{ type: 'text', text: 'hello' }] },
+        ],
+      },
+    });
+
+    // OUT of the set ----------------------------------------------------------
+    // (d) non-deleted, text_content NULL, no embeddings.
+    await insertPage({ textContent: null });
+    // (e) non-deleted, whitespace-only text (regex requires a non-space char).
+    await insertPage({ textContent: '   \n\t  ' });
+    // (f) deleted page WITH body text — excluded by the non-deleted predicate.
+    await insertPage({
+      textContent: 'deleted but had text',
+      deletedAt: new Date(),
+    });
+    // (g) non-deleted, no text, with ONLY a DELETED embedding row — the EXISTS
+    //     subquery filters pe.deleted_at IS NULL, so this stays out.
+    const onlyDeletedEmbedding = await insertPage({ textContent: null });
+    await insertEmbedding(onlyDeletedEmbedding, { deletedAt: new Date() });
+    // (h) non-deleted, empty text_content, content JSON with ONLY a math atom
+    //     node — its LaTeX lives in `attrs.text` (a `"text":` KEY, not a
+    //     `"type":"text"` text node) and has no text serializer, so `jsonToText`
+    //     yields nothing and the page produces zero embeddings. The predicate
+    //     keys on the structural `"type":"text"` marker, so this stays OUT (a
+    //     bare `"text":` match would wrongly inflate the denominator).
+    await insertPage({
+      textContent: null,
+      content: {
+        type: 'doc',
+        content: [{ type: 'mathBlock', attrs: { text: 'E=mc^2' } }],
+      },
+    });
+
+    const ids = await repo.getEmbeddablePageIds(workspaceId);
+    const count = await repo.countEmbeddablePages(workspaceId);
+
+    // The two queries agree on the size (the load-bearing lockstep invariant)...
+    expect(ids.length).toBe(count);
+    // ...and the set is exactly the three qualifying pages, nothing else.
+    expect(new Set(ids)).toEqual(
+      new Set([withText, noTextLiveEmbedding, noTextContentDoc]),
+    );
+    expect(count).toBe(3);
+  });
+});

packages/editor-ext/src/index.ts

@@ -25,7 +25,6 @@ export * from "./lib/subpages";
 export * from "./lib/transclusion";
 export * from "./lib/page-embed";
 export * from "./lib/highlight";
-export * from "./lib/spoiler/spoiler";
 export * from "./lib/indent";
 export * from "./lib/heading/heading";
 export * from "./lib/unique-id";

packages/editor-ext/src/lib/markdown/utils/spoiler.marked.test.ts

@@ -1,128 +0,0 @@
-import { describe, it, expect } from "vitest";
-import { getSchema } from "@tiptap/core";
-import { generateHTML, generateJSON } from "@tiptap/html";
-import { Document } from "@tiptap/extension-document";
-import { Paragraph } from "@tiptap/extension-paragraph";
-import { Text } from "@tiptap/extension-text";
-import { Bold } from "@tiptap/extension-bold";
-import { htmlToMarkdown } from "./turndown.utils";
-import { markdownToHtml } from "./marked.utils";
-import { Spoiler } from "../../spoiler/spoiler";
-
-// The spoiler mark has no native Markdown syntax, so it is preserved losslessly
-// as raw inline HTML (`<span data-spoiler="true">…</span>`), the same approach
-// htmlEmbed uses. This test drives the full editor round-trip:
-//   JSON -> HTML -> Markdown -> HTML -> JSON
-// and asserts the `spoiler` mark survives end to end. We use the same
-// getSchema + @tiptap/html generateHTML/generateJSON utilities the other
-// editor-ext schema tests use.
-
-const extensions = [Document, Paragraph, Text, Bold, Spoiler];
-
-function html(md: string): string {
-  const out = markdownToHtml(md);
-  if (typeof out !== "string") throw new Error("expected sync string output");
-  return out;
-}
-
-// Count text nodes carrying a `spoiler` mark anywhere in a ProseMirror JSON doc.
-function countSpoilerMarks(doc: any): number {
-  let count = 0;
-  const walk = (node: any) => {
-    if (!node || typeof node !== "object") return;
-    if (Array.isArray(node.marks)) {
-      for (const mark of node.marks) {
-        if (mark?.type === "spoiler") count++;
-      }
-    }
-    if (Array.isArray(node.content)) node.content.forEach(walk);
-  };
-  walk(doc);
-  return count;
-}
-
-describe("Spoiler mark schema", () => {
-  it("registers the spoiler mark in the schema", () => {
-    const schema = getSchema(extensions);
-    expect(schema.marks.spoiler).toBeTruthy();
-  });
-
-  it("recovers the spoiler mark from span[data-spoiler] (HTML -> JSON)", () => {
-    const json = generateJSON(
-      '<p>before <span data-spoiler="true">hidden</span> after</p>',
-      extensions,
-    );
-    expect(countSpoilerMarks(json)).toBe(1);
-  });
-
-  it("emits data-spoiler + class on render (JSON -> HTML)", () => {
-    const doc = {
-      type: "doc",
-      content: [
-        {
-          type: "paragraph",
-          content: [
-            {
-              type: "text",
-              text: "hidden",
-              marks: [{ type: "spoiler" }],
-            },
-          ],
-        },
-      ],
-    };
-    const out = generateHTML(doc, extensions);
-    expect(out).toContain('data-spoiler="true"');
-    expect(out).toContain('class="spoiler"');
-  });
-});
-
-describe("Spoiler Markdown round-trip is lossless", () => {
-  const docWith = (textNode: any) => ({
-    type: "doc",
-    content: [
-      {
-        type: "paragraph",
-        content: [{ type: "text", text: "before " }, textNode, { type: "text", text: " after" }],
-      },
-    ],
-  });
-
-  it("preserves the spoiler mark through JSON -> MD -> HTML -> JSON", () => {
-    const startDoc = docWith({
-      type: "text",
-      text: "hidden",
-      marks: [{ type: "spoiler" }],
-    });
-
-    // JSON -> HTML
-    const html1 = generateHTML(startDoc, extensions);
-    expect(html1).toContain('data-spoiler="true"');
-
-    // HTML -> Markdown (raw inline HTML, lossless)
-    const md = htmlToMarkdown(html1);
-    expect(md).toContain('<span data-spoiler="true">hidden</span>');
-
-    // MD -> HTML -> JSON (mark restored via parseHTML)
-    const endJson = generateJSON(html(md), extensions);
-    expect(countSpoilerMarks(endJson)).toBe(1);
-    // The visible text survives.
-    expect(JSON.stringify(endJson)).toContain("hidden");
-  });
-
-  it("keeps the spoiler intact when it intersects a bold mark", () => {
-    const startDoc = docWith({
-      type: "text",
-      text: "secret",
-      marks: [{ type: "bold" }, { type: "spoiler" }],
-    });
-
-    const md = htmlToMarkdown(generateHTML(startDoc, extensions));
-    expect(md).toContain("data-spoiler=\"true\"");
-
-    const endJson = generateJSON(html(md), extensions);
-    expect(countSpoilerMarks(endJson)).toBe(1);
-    // Bold survives alongside the spoiler.
-    expect(JSON.stringify(endJson)).toContain('"bold"');
-  });
-});

packages/editor-ext/src/lib/markdown/utils/turndown.dataloss.test.ts

@@ -1,77 +1,147 @@
 import { describe, it, expect } from "vitest";
 import { htmlToMarkdown } from "./turndown.utils";
+import { markdownToHtml } from "./marked.utils";
 
 /**
- * #206 mdrt-2 — Markdown export must never SILENTLY drop a block.
+ * #206 mdrt-2 — Markdown export must never SILENTLY drop a block. (FIXED)
  *
- * `htmlToMarkdown` (turndown) only registers rules for a fixed set of custom
- * nodes (callout, taskItem, details, math, iframe, htmlEmbed, image, video,
- * footnote). Any other custom node — `transclusionReference`, `pageBreak`,
- * `mention`, `status` — falls through to turndown's default handling: an empty
- * wrapper is "blank" and removed, so the block disappears from the exported
- * Markdown with no trace. The invariant "never silently lose a block" is broken.
+ * `htmlToMarkdown` (turndown) historically only registered rules for a fixed
+ * set of custom nodes (callout, taskItem, details, math, iframe, htmlEmbed,
+ * image, video, footnote). Any other custom node — `transclusionReference`,
+ * `pageBreak`, `mention`, `status` — fell through to turndown's default
+ * handling: an empty wrapper is "blank" and removed, so the block disappeared
+ * from the exported Markdown with no trace, and `mention`/`status` collapsed to
+ * bare text, losing their identity (data-id / data-color). The invariant
+ * "never silently lose a block" was broken.
  *
- * The `it.fails` cases assert the DESIRED contract (the block survives export in
- * SOME form) and are RED today: they document the unfixed data loss and flip to
- * green the moment a turndown rule (real syntax or a lossless HTML-comment
- * placeholder) is added. A normal characterization `it` pins the exact current
- * lossy output so the regression is unambiguous.
+ * The fix adds lossless turndown rules that re-emit each of these nodes as raw
+ * HTML carrying every `data-*` attribute. Plain-Markdown viewers ignore the
+ * inert tag; the import path round-trips it (`markdownToHtml` passes the raw
+ * HTML through and each node's `parseHTML` rebuilds the ProseMirror node). These
+ * tests assert the surviving contract (the block is preserved AND its identity
+ * round-trips back through import).
  */
-describe("htmlToMarkdown — custom nodes without a turndown rule (#206 mdrt-2)", () => {
-  const wrap = (inner: string) =>
-    `<p>before</p>${inner}<p>after</p>`;
+describe("htmlToMarkdown — custom nodes are preserved losslessly (#206 mdrt-2)", () => {
+  const wrap = (inner: string) => `<p>before</p>${inner}<p>after</p>`;
 
-  it("CURRENTLY drops a pageBreak entirely (data loss)", () => {
+  it("preserves a pageBreak block on Markdown export", () => {
     const md = htmlToMarkdown(
       wrap('<div data-type="pageBreak" class="page-break"></div>'),
     );
-    // The page break vanishes: only the two paragraphs remain, nothing between.
     expect(md).toContain("before");
     expect(md).toContain("after");
-    expect(md).not.toMatch(/page-?break/i);
-    expect(md).not.toContain("---"); // not even a horizontal-rule fallback
+    // The break survives as an inert raw-HTML tag, not silently dropped.
+    expect(md).toMatch(/data-type="pageBreak"/);
+    expect(md).toMatch(/page-?break/i);
   });
 
-  it("CURRENTLY drops a transclusionReference entirely (data loss)", () => {
+  it("preserves a transclusionReference's identity on Markdown export", () => {
     const md = htmlToMarkdown(
       wrap('<div data-type="transclusionReference" data-id="abc"></div>'),
     );
     expect(md).toContain("before");
     expect(md).toContain("after");
-    // The data-id (the only thing that gives the reference identity) is gone.
-    expect(md).not.toContain("abc");
+    // The data-id (the only thing that gives the reference identity) survives.
+    expect(md).toContain("abc");
+    expect(md).toMatch(/data-type="transclusionReference"/);
   });
 
-  it.fails(
-    "should NOT lose a pageBreak block on Markdown export",
-    () => {
+  it("preserves a mention's data-id (stable identity) on Markdown export", () => {
+    const md = htmlToMarkdown(
+      '<p>hi <span data-type="mention" data-id="u1" data-label="Bob">@Bob</span> there</p>',
+    );
+    // The mention keeps its stable identity (data-id), not just the text.
+    expect(md).toContain("u1");
+    expect(md).toContain("Bob");
+    expect(md).toMatch(/data-type="mention"/);
+  });
+
+  it("preserves a status chip's color on Markdown export", () => {
+    const md = htmlToMarkdown(
+      '<p>s <span data-type="status" data-color="green">Done</span></p>',
+    );
+    // The chip's color (its identity) survives, not just the visible text.
+    expect(md).toContain("green");
+    expect(md).toContain("Done");
+    expect(md).toMatch(/data-type="status"/);
+  });
+
+  // The export form is only lossless if the import path can rebuild it. These
+  // assert the full MD -> HTML round-trip restores the node + its attributes,
+  // which is the marker <-> node contract each `parseHTML` relies on.
+  describe("import round-trip (markdownToHtml restores the node)", () => {
+    it("round-trips a pageBreak through export + import", async () => {
       const md = htmlToMarkdown(
         wrap('<div data-type="pageBreak" class="page-break"></div>'),
       );
-      // Desired: the break survives in some form (e.g. a `---` rule or marker).
-      expect(md).toMatch(/(-{3,}|page-?break)/i);
-    },
-  );
+      const html = await markdownToHtml(md);
+      expect(html).toMatch(/<div[^>]*data-type="pageBreak"[^>]*>/);
+      expect(html).toContain("before");
+      expect(html).toContain("after");
+    });
 
-  it.fails(
-    "should NOT lose a transclusionReference's identity on Markdown export",
-    () => {
+    it("round-trips a transclusionReference (keeps data-id)", async () => {
       const md = htmlToMarkdown(
         wrap('<div data-type="transclusionReference" data-id="abc"></div>'),
       );
-      // Desired: the referenced id survives so the block can be rebuilt.
-      expect(md).toContain("abc");
-    },
-  );
+      const html = await markdownToHtml(md);
+      expect(html).toMatch(/<div[^>]*data-type="transclusionReference"[^>]*>/);
+      expect(html).toContain("abc");
+    });
 
-  it.fails(
-    "should NOT lose a mention's data-id on Markdown export",
-    () => {
+    it("round-trips a mention (keeps data-id + data-label)", async () => {
       const md = htmlToMarkdown(
         '<p>hi <span data-type="mention" data-id="u1" data-label="Bob">@Bob</span> there</p>',
       );
-      // Desired: the mention keeps its stable identity (data-id), not just text.
-      expect(md).toContain("u1");
-    },
-  );
+      const html = await markdownToHtml(md);
+      expect(html).toMatch(/<span[^>]*data-type="mention"[^>]*>/);
+      expect(html).toContain("u1");
+      expect(html).toContain("Bob");
+    });
+
+    it("round-trips a status chip (keeps data-color)", async () => {
+      const md = htmlToMarkdown(
+        '<p>s <span data-type="status" data-color="green">Done</span></p>',
+      );
+      const html = await markdownToHtml(md);
+      expect(html).toMatch(/<span[^>]*data-type="status"[^>]*>/);
+      expect(html).toContain("green");
+    });
+
+    // HTML special chars in an attribute value or in a node's text must be
+    // ESCAPED when re-emitted as raw HTML, otherwise the exported tag is
+    // malformed and `markdownToHtml`'s parser cannot restore the original value
+    // (the same silent data loss this PR fixes). Dropping `<`/`>` escaping is the
+    // dangerous regression: a stray `<` or `>` corrupts the tag (or injects new
+    // markup), so the test data carries ALL of `&`, `"`, `<`, `>` in BOTH the
+    // data-label attribute and the visible text. That fully exercises
+    // escapeHtmlAttr's `&,",<,>` branches and escapeHtmlText's `&,<,>` branches
+    // (escapeHtmlText leaves `"` literal); the alphanumeric-only cases above hit
+    // none of them.
+    it("escapes HTML special chars (& \" < >) in attrs + text and round-trips them", async () => {
+      const md = htmlToMarkdown(
+        `<p>hi <span data-type="mention" data-id="u1" data-label="A &amp; &lt;B&gt; &quot;C&quot;">@A &amp; &lt;B&gt; "C"</span> there</p>`,
+      );
+
+      // (a) The exported Markdown carries a WELL-FORMED, correctly-escaped tag:
+      // the attribute escapes `&`, `<`, `>` AND `"`; the text escapes `&`, `<`,
+      // `>` (a `"` inside text content is legal, so it stays literal).
+      expect(md).toContain('data-label="A &amp; &lt;B&gt; &quot;C&quot;"');
+      expect(md).toContain('>@A &amp; &lt;B&gt; "C"</span>');
+      // And explicitly NOT the raw, tag-corrupting forms: a literal `<B>` (would
+      // mean `<`/`>` escaping was dropped in either the attr or the text)...
+      expect(md).not.toContain("<B>");
+      // ...nor the malformed attribute that an unescaped `"` would produce.
+      expect(md).not.toContain('data-label="A &amp; &lt;B&gt; "C""');
+
+      // (b) Import restores the ORIGINAL (unescaped) values, attribute and text.
+      const html = await markdownToHtml(md);
+      const dom = new DOMParser().parseFromString(html as string, "text/html");
+      const span = dom.querySelector('span[data-type="mention"]');
+      expect(span).not.toBeNull();
+      expect(span!.getAttribute("data-id")).toBe("u1");
+      expect(span!.getAttribute("data-label")).toBe('A & <B> "C"');
+      expect(span!.textContent).toBe('@A & <B> "C"');
+    });
+  });
 });

packages/editor-ext/src/lib/markdown/utils/turndown.utils.ts

@@ -43,6 +43,54 @@ function fillEmptyFootnoteRefs(html: string): string {
   );
 }
 
+/**
+ * `pageBreak` and `transclusionReference` are childless atom <div>s. Like an
+ * empty footnote ref (see above), turndown treats a childless block as "blank"
+ * and replaces it with the blankRule BEFORE any custom rule can fire — so the
+ * node disappears from the export with no trace (#206 mdrt-2). Inject a
+ * zero-width space so the node is non-blank and our lossless rule runs; the
+ * rule rebuilds the tag from the element's attributes, so the injected char
+ * never reaches the output.
+ */
+function fillEmptyAtomBlocks(html: string): string {
+  return html.replace(
+    /<div\b([^>]*\bdata-type="(?:pageBreak|transclusionReference)"[^>]*)>\s*<\/div>/gi,
+    (_m, attrs) => `<div${attrs}>​</div>`,
+  );
+}
+
+/** HTML-escape an attribute value so a re-emitted raw-HTML tag is well-formed. */
+function escapeHtmlAttr(value: string): string {
+  return value
+    .replace(/&/g, '&amp;')
+    .replace(/"/g, '&quot;')
+    .replace(/</g, '&lt;')
+    .replace(/>/g, '&gt;');
+}
+
+/** HTML-escape text placed inside a re-emitted raw-HTML element. */
+function escapeHtmlText(value: string): string {
+  return value
+    .replace(/&/g, '&amp;')
+    .replace(/</g, '&lt;')
+    .replace(/>/g, '&gt;');
+}
+
+/**
+ * Serialize ALL of an element's attributes back to a raw-HTML attribute string
+ * (leading space included). Generic on purpose: a custom node's identity lives
+ * entirely in its `data-*` attributes (data-id, data-color, data-source-page-id,
+ * data-transclusion-id, …), and serializing every attribute keeps the export
+ * lossless regardless of which attributes a given node carries.
+ */
+function serializeAttrs(node: any): string {
+  const attrs = node?.attributes;
+  if (!attrs) return '';
+  return Array.from(attrs as ArrayLike<{ name: string; value: string }>)
+    .map((attr) => ` ${attr.name}="${escapeHtmlAttr(attr.value ?? '')}"`)
+    .join('');
+}
+
 export function htmlToMarkdown(html: string): string {
   const turndownService = new TurndownService({
     headingStyle: 'atx',
@@ -65,17 +113,87 @@ export function htmlToMarkdown(html: string): string {
     mathBlock,
     iframeEmbed,
     htmlEmbed,
-    spoiler,
     image,
     video,
     footnoteReference,
     footnotesList,
+    pageBreak,
+    transclusionReference,
+    mention,
+    status,
   ]);
   return turndownService
-    .turndown(fillEmptyFootnoteRefs(html))
+    .turndown(fillEmptyAtomBlocks(fillEmptyFootnoteRefs(html)))
     .replaceAll('<br>', ' ');
 }
 
+/**
+ * Lossless export rules for custom nodes that have NO native Markdown syntax
+ * (#206 mdrt-2). Markdown cannot represent a page break, a transclusion
+ * reference, a mention's stable id, or a status chip's color — so rather than
+ * letting turndown silently drop them, each rule re-emits the node as raw HTML
+ * carrying every `data-*` attribute. Plain-Markdown viewers ignore the inert
+ * tag, and the import path round-trips it: `markdownToHtml` passes raw HTML
+ * through and each node's `parseHTML` (`div[data-type="…"]`, `span[…]`) rebuilds
+ * the ProseMirror node with its attributes intact.
+ */
+function pageBreak(turndownService: _TurndownService) {
+  turndownService.addRule('pageBreak', {
+    filter: function (node: HTMLInputElement) {
+      return (
+        node.nodeName === 'DIV' &&
+        node.getAttribute('data-type') === 'pageBreak'
+      );
+    },
+    replacement: function (_content: string, node: HTMLInputElement) {
+      return `\n\n<div${serializeAttrs(node)}></div>\n\n`;
+    },
+  });
+}
+
+function transclusionReference(turndownService: _TurndownService) {
+  turndownService.addRule('transclusionReference', {
+    filter: function (node: HTMLInputElement) {
+      return (
+        node.nodeName === 'DIV' &&
+        node.getAttribute('data-type') === 'transclusionReference'
+      );
+    },
+    replacement: function (_content: string, node: HTMLInputElement) {
+      return `\n\n<div${serializeAttrs(node)}></div>\n\n`;
+    },
+  });
+}
+
+function mention(turndownService: _TurndownService) {
+  turndownService.addRule('mention', {
+    filter: function (node: HTMLInputElement) {
+      return (
+        node.nodeName === 'SPAN' &&
+        node.getAttribute('data-type') === 'mention'
+      );
+    },
+    replacement: function (_content: string, node: HTMLInputElement) {
+      const text = escapeHtmlText(node.textContent || '');
+      return `<span${serializeAttrs(node)}>${text}</span>`;
+    },
+  });
+}
+
+function status(turndownService: _TurndownService) {
+  turndownService.addRule('status', {
+    filter: function (node: HTMLInputElement) {
+      return (
+        node.nodeName === 'SPAN' && node.getAttribute('data-type') === 'status'
+      );
+    },
+    replacement: function (_content: string, node: HTMLInputElement) {
+      const text = escapeHtmlText(node.textContent || '');
+      return `<span${serializeAttrs(node)}>${text}</span>`;
+    },
+  });
+}
+
 /**
  * Serialize the `htmlEmbed` node to Markdown.
  *
@@ -102,29 +220,6 @@ function htmlEmbed(turndownService: _TurndownService) {
   });
 }
 
-/**
- * Serialize the `spoiler` inline mark to lossless raw inline HTML.
- *
- * Markdown has no native spoiler syntax, so we emit the same `<span
- * data-spoiler="true">…</span>` the mark renders. `marked` passes inline raw HTML
- * through untouched, and `generateJSON` restores the mark via its parseHTML, so
- * the round-trip MD -> HTML -> JSON keeps the spoiler intact. The UI-only
- * `is-revealed` state is never serialized.
- */
-function spoiler(turndownService: _TurndownService) {
-  turndownService.addRule('spoiler', {
-    filter: function (node: HTMLInputElement) {
-      return (
-        node.nodeName === 'SPAN' &&
-        node.getAttribute('data-spoiler') === 'true'
-      );
-    },
-    replacement: function (content: string) {
-      return `<span data-spoiler="true">${content}</span>`;
-    },
-  });
-}
-
 function listParagraph(turndownService: _TurndownService) {
   turndownService.addRule('paragraph', {
     filter: ['p'],

packages/editor-ext/src/lib/recreate-transform/recreateTransform.test.ts

@@ -0,0 +1,133 @@
+import { describe, it, expect } from "vitest";
+import { schema } from "@tiptap/pm/schema-basic";
+import type { Node as PMNode } from "@tiptap/pm/model";
+import { Transform } from "@tiptap/pm/transform";
+import { recreateTransform } from "./recreateTransform";
+
+/**
+ * recreateTransform diffs two documents and produces ProseMirror steps that turn
+ * `fromDoc` into `toDoc`. It is the backbone of collaborative/version diffing, so
+ * THE invariant that matters is: replaying the produced steps on `fromDoc` must
+ * reproduce `toDoc` exactly. Every test below re-applies the steps onto a fresh
+ * Transform seeded from `fromDoc` (not just trusting `tr.doc`) and asserts node
+ * equality with `.eq()`. If a regression makes any step wrong, the round-trip
+ * breaks and the test fails.
+ */
+
+// Real ProseMirror schema (the standard basic schema) with paragraph/heading +
+// strong/em marks — the same primitives the editor diffs in production.
+const doc = (...c: PMNode[]) => schema.node("doc", null, c);
+const p = (...c: PMNode[]) =>
+  schema.node("paragraph", null, c.length ? c : undefined);
+const h = (level: number, ...c: PMNode[]) =>
+  schema.node("heading", { level }, c);
+const t = (text: string, ...marks: any[]) =>
+  schema.text(text, marks.length ? marks : undefined);
+const strong = schema.marks.strong.create();
+const em = schema.marks.em.create();
+
+// Replay the diff's steps onto a fresh Transform built from `fromDoc`. This is
+// the faithful "apply(diff) == target" check — it exercises the actual Step
+// objects rather than the transform's internal accumulated doc.
+function applyDiff(fromDoc: PMNode, toDoc: PMNode, options?: any): PMNode {
+  const tr = recreateTransform(fromDoc, toDoc, options);
+  const replay = new Transform(fromDoc);
+  tr.steps.forEach((s) => {
+    const result = replay.maybeStep(s);
+    if (result.failed) throw new Error(`step failed: ${result.failed}`);
+  });
+  return replay.doc;
+}
+
+describe("recreateTransform round-trip (apply(diff) == target)", () => {
+  it("reconstructs the target on plain text insertion", () => {
+    // Inserting " world" must yield exactly the target paragraph.
+    const from = doc(p(t("hello")));
+    const to = doc(p(t("hello world")));
+    expect(applyDiff(from, to).eq(to)).toBe(true);
+  });
+
+  it("reconstructs the target on text deletion", () => {
+    // Deleting a trailing word is the inverse of insertion and must round-trip.
+    const from = doc(p(t("hello world")));
+    const to = doc(p(t("hello")));
+    expect(applyDiff(from, to).eq(to)).toBe(true);
+  });
+
+  it("reconstructs the target when a word is replaced mid-string", () => {
+    // A char-level replace in the middle must not corrupt the surrounding text.
+    const from = doc(p(t("the quick brown fox")));
+    const to = doc(p(t("the slow brown fox")));
+    expect(applyDiff(from, to).eq(to)).toBe(true);
+  });
+
+  it("reconstructs the target when a mark is added (complexSteps path)", () => {
+    // Mark-only changes are diffed in a separate pass; the bolded run must match.
+    const from = doc(p(t("hello")));
+    const to = doc(p(t("hello", strong)));
+    const out = applyDiff(from, to);
+    expect(out.eq(to)).toBe(true);
+    // Sanity: the produced doc actually carries the strong mark.
+    expect(out.firstChild!.firstChild!.marks.length).toBe(1);
+  });
+
+  it("reconstructs the target when a mark is removed", () => {
+    // Removing the only mark must leave the same text with no marks.
+    const from = doc(p(t("hello", strong)));
+    const to = doc(p(t("hello")));
+    const out = applyDiff(from, to);
+    expect(out.eq(to)).toBe(true);
+    expect(out.firstChild!.firstChild!.marks.length).toBe(0);
+  });
+
+  it("reconstructs the target on a paragraph split into two blocks", () => {
+    // Structural change (one block -> two) must replay as valid replace steps.
+    const from = doc(p(t("hello world")));
+    const to = doc(p(t("hello")), p(t("world")));
+    const out = applyDiff(from, to);
+    expect(out.eq(to)).toBe(true);
+    expect(out.childCount).toBe(2);
+  });
+
+  it("reconstructs the target on a node-type change (paragraph -> heading)", () => {
+    // Type/attrs changes drive the setNodeMarkup branch; the node must become a
+    // heading while keeping its text.
+    const from = doc(p(t("hello")));
+    const to = doc(h(1, t("hello")));
+    const out = applyDiff(from, to);
+    expect(out.eq(to)).toBe(true);
+    expect(out.firstChild!.type.name).toBe("heading");
+  });
+
+  it("reconstructs a combined structural + mark change", () => {
+    // Several diff kinds at once (new block + italic run) still round-trips.
+    const from = doc(p(t("alpha")));
+    const to = doc(p(t("alpha")), p(t("beta", em)));
+    const out = applyDiff(from, to);
+    expect(out.eq(to)).toBe(true);
+  });
+
+  it("produces an empty step list for identical documents", () => {
+    // No diff => no work; spurious steps would mean wasted/incorrect history.
+    const from = doc(p(t("same")));
+    const to = doc(p(t("same")));
+    const tr = recreateTransform(from, to);
+    expect(tr.steps.length).toBe(0);
+    expect(tr.doc.eq(to)).toBe(true);
+  });
+
+  it("round-trips with complexSteps:false (marks diffed as replaces)", () => {
+    // With complexSteps off, mark changes are folded into replace steps rather
+    // than dedicated mark steps — the result must still equal the target.
+    const from = doc(p(t("hello")));
+    const to = doc(p(t("hello", strong)));
+    expect(applyDiff(from, to, { complexSteps: false }).eq(to)).toBe(true);
+  });
+
+  it("round-trips with wordDiffs:true (whole-word text diffing)", () => {
+    // wordDiffs changes the granularity of the text diff, not the outcome.
+    const from = doc(p(t("the quick brown fox")));
+    const to = doc(p(t("the quick red fox")));
+    expect(applyDiff(from, to, { wordDiffs: true }).eq(to)).toBe(true);
+  });
+});

packages/editor-ext/src/lib/spoiler/spoiler.ts

@@ -1,74 +0,0 @@
-import { Mark, markInputRule, mergeAttributes } from "@tiptap/core";
-
-export interface SpoilerOptions {
-  HTMLAttributes: Record<string, unknown>;
-}
-
-declare module "@tiptap/core" {
-  interface Commands<ReturnType> {
-    spoiler: {
-      setSpoiler: () => ReturnType;
-      toggleSpoiler: () => ReturnType;
-      unsetSpoiler: () => ReturnType;
-    };
-  }
-}
-
-// Discord-style `||text||` input rule. Requires a non-space right after the
-// opening `||` and a non-space right before the closing `||` so empty/padded
-// markers don't match.
-const inputRegex = /(?:^|\s)(\|\|(?!\s)([^|]+)(?<!\s)\|\|)$/;
-
-export const Spoiler = Mark.create<SpoilerOptions>({
-  name: "spoiler",
-
-  // Don't bleed onto text typed at the boundary (mirrors link).
-  inclusive: false,
-
-  addOptions() {
-    return {
-      HTMLAttributes: {},
-    };
-  },
-
-  parseHTML() {
-    return [{ tag: "span[data-spoiler]" }];
-  },
-
-  renderHTML({ HTMLAttributes }) {
-    return [
-      "span",
-      mergeAttributes(this.options.HTMLAttributes, HTMLAttributes, {
-        "data-spoiler": "true",
-        class: "spoiler",
-      }),
-      0,
-    ];
-  },
-
-  addCommands() {
-    return {
-      setSpoiler:
-        () =>
-        ({ commands }) =>
-          commands.setMark(this.name),
-      toggleSpoiler:
-        () =>
-        ({ commands }) =>
-          commands.toggleMark(this.name),
-      unsetSpoiler:
-        () =>
-        ({ commands }) =>
-          commands.unsetMark(this.name),
-    };
-  },
-
-  addInputRules() {
-    return [markInputRule({ find: inputRegex, type: this.type })];
-  },
-
-  // No addKeyboardShortcuts: the issue's proposed `Mod-Shift-s` is already taken
-  // by the built-in Strike mark (and `Mod-Shift-h` by Highlight). The `||text||`
-  // input rule plus the bubble-menu button cover ergonomics, so we omit a hotkey
-  // rather than collide with an existing one.
-});

packages/editor-ext/src/lib/table/utils/get-selection-range-in-column.test.ts

@@ -0,0 +1,75 @@
+import { describe, it, expect } from "vitest";
+import { getSelectionRangeInColumn } from "./get-selection-range-in-column";
+import { cell, row, table, doc, trFor } from "./table-test-helpers";
+
+/**
+ * getSelectionRangeInColumn computes the rectangular column range (the set of
+ * column indexes, plus anchor/head cell positions) that a drag-reorder or
+ * column-select operation should act on, accounting for merged (colspan) cells.
+ * It keys off the table found from the current selection, so we drive it with a
+ * real EditorState whose selection sits inside the table.
+ */
+
+// A 2-row x 3-col grid; each column is identifiable by its top-row letter.
+const grid3x2 = () =>
+  doc(
+    table(
+      row(cell("a"), cell("b"), cell("c")),
+      row(cell("d"), cell("e"), cell("f")),
+    ),
+  );
+
+describe("getSelectionRangeInColumn", () => {
+  it("returns a single-column range for a single index", () => {
+    // Asking for column 1 yields exactly indexes [1].
+    const tr = trFor(grid3x2());
+    const range = getSelectionRangeInColumn(tr, 1);
+    expect(range).toBeTruthy();
+    expect(range!.indexes).toEqual([1]);
+  });
+
+  it("anchor/head resolve to the top and bottom cells OF the requested column", () => {
+    // $head must point at the column's first (top) cell and $anchor at its last
+    // (bottom) cell — pinning that the returned positions belong to column 1,
+    // not some other column.
+    const tr = trFor(grid3x2());
+    const range = getSelectionRangeInColumn(tr, 1)!;
+    expect(tr.doc.nodeAt(range.$head.pos)?.textContent).toBe("b"); // top of col 1
+    expect(tr.doc.nodeAt(range.$anchor.pos)?.textContent).toBe("e"); // bottom of col 1
+  });
+
+  it("returns the inclusive span of columns for a multi-column request", () => {
+    // A 0..2 request must enumerate every covered column, in order.
+    const tr = trFor(grid3x2());
+    const range = getSelectionRangeInColumn(tr, 0, 2);
+    expect(range!.indexes).toEqual([0, 1, 2]);
+  });
+
+  it("returns a two-column span for an adjacent pair", () => {
+    const tr = trFor(grid3x2());
+    const range = getSelectionRangeInColumn(tr, 1, 2);
+    expect(range!.indexes).toEqual([1, 2]);
+  });
+
+  it("expands the range to cover a horizontally merged (colspan) cell", () => {
+    // Row 0 col 0 spans 2 columns. Requesting just column 0 must pull column 1
+    // into the range because they are merged together in the top row.
+    const d = doc(
+      table(
+        row(cell("ab", { colspan: 2 }), cell("c")),
+        row(cell("d"), cell("e"), cell("f")),
+      ),
+    );
+    const tr = trFor(d);
+    const range = getSelectionRangeInColumn(tr, 0);
+    expect(range!.indexes).toEqual([0, 1]);
+  });
+
+  it("throws when the requested column is entirely out of range", () => {
+    // No cells exist at column 5 of a 3-wide table, so the function cannot pick
+    // an anchor cell and dereferences undefined — pin this as the current
+    // (caller-guarded) contract so a silent behavior change is caught.
+    const tr = trFor(grid3x2());
+    expect(() => getSelectionRangeInColumn(tr, 5)).toThrow();
+  });
+});

packages/editor-ext/src/lib/table/utils/move-column.test.ts

@@ -0,0 +1,127 @@
+import { describe, it, expect } from "vitest";
+import { CellSelection } from "@tiptap/pm/tables";
+import { moveColumn } from "./move-column";
+import {
+  schema,
+  cell,
+  row,
+  table,
+  doc,
+  grid,
+  stateFor,
+} from "./table-test-helpers";
+
+/**
+ * moveColumn reorders whole columns of a real ProseMirror table by mutating a
+ * Transaction (transpose -> move row -> transpose back -> replace). The invariant
+ * is that after the call each column appears at its new position with every
+ * cell's content preserved and nothing dropped or duplicated.
+ */
+
+// 2-row x 3-col table; column k is (rowX-col-k). Columns: 0=(a,d) 1=(b,e) 2=(c,f).
+const grid3x2 = () =>
+  doc(
+    table(
+      row(cell("a"), cell("b"), cell("c")),
+      row(cell("d"), cell("e"), cell("f")),
+    ),
+  );
+
+describe("moveColumn", () => {
+  it("moves the first column to the last index, preserving column content", () => {
+    // origin 0 -> target 2 sends column (a,d) to the right: cols become 1,2,0.
+    const state = stateFor(grid3x2());
+    const tr = state.tr;
+    const ok = moveColumn({
+      tr,
+      originIndex: 0,
+      targetIndex: 2,
+      select: false,
+      pos: state.selection.from,
+    });
+    expect(ok).toBe(true);
+    expect(grid(tr)).toEqual([
+      ["b", "c", "a"],
+      ["e", "f", "d"],
+    ]);
+  });
+
+  it("moves a later column to the first index", () => {
+    // origin 2 -> target 0 pulls column (c,f) to the front: cols become 2,0,1.
+    const state = stateFor(grid3x2());
+    const tr = state.tr;
+    const ok = moveColumn({
+      tr,
+      originIndex: 2,
+      targetIndex: 0,
+      select: false,
+      pos: state.selection.from,
+    });
+    expect(ok).toBe(true);
+    expect(grid(tr)).toEqual([
+      ["c", "a", "b"],
+      ["f", "d", "e"],
+    ]);
+  });
+
+  it("never drops or duplicates cells when reordering columns", () => {
+    const state = stateFor(grid3x2());
+    const tr = state.tr;
+    moveColumn({
+      tr,
+      originIndex: 1,
+      targetIndex: 2,
+      select: false,
+      pos: state.selection.from,
+    });
+    expect(grid(tr).flat().sort()).toEqual(
+      ["a", "b", "c", "d", "e", "f"].sort(),
+    );
+    expect(grid(tr)[0].length).toBe(3);
+  });
+
+  it("returns false (no-op) when target equals origin", () => {
+    const state = stateFor(grid3x2());
+    const tr = state.tr;
+    const before = grid(tr);
+    const ok = moveColumn({
+      tr,
+      originIndex: 1,
+      targetIndex: 1,
+      select: false,
+      pos: state.selection.from,
+    });
+    expect(ok).toBe(false);
+    expect(grid(tr)).toEqual(before);
+  });
+
+  it("returns false when pos is not inside a table", () => {
+    const d = doc(
+      schema.nodes.paragraph.createChecked(null, schema.text("plain")),
+    );
+    const state = stateFor(d);
+    const tr = state.tr;
+    const ok = moveColumn({
+      tr,
+      originIndex: 0,
+      targetIndex: 1,
+      select: false,
+      pos: state.selection.from,
+    });
+    expect(ok).toBe(false);
+  });
+
+  it("installs a CellSelection on the moved column when select is true", () => {
+    const state = stateFor(grid3x2());
+    const tr = state.tr;
+    const ok = moveColumn({
+      tr,
+      originIndex: 0,
+      targetIndex: 2,
+      select: true,
+      pos: state.selection.from,
+    });
+    expect(ok).toBe(true);
+    expect(tr.selection instanceof CellSelection).toBe(true);
+  });
+});

packages/editor-ext/src/lib/table/utils/move-row.test.ts

@@ -0,0 +1,136 @@
+import { describe, it, expect } from "vitest";
+import { CellSelection } from "@tiptap/pm/tables";
+import { moveRow } from "./move-row";
+import {
+  schema,
+  cell,
+  row,
+  table,
+  doc,
+  grid,
+  stateFor,
+} from "./table-test-helpers";
+
+/**
+ * moveRow reorders whole rows of a real ProseMirror table by mutating a
+ * Transaction: it locates the table, computes origin/target row ranges, rebuilds
+ * the table with rows reordered, and replaces it in the doc. The invariant is
+ * that after the call the table's rows appear in the new order with every cell's
+ * content preserved, and no rows are dropped or duplicated.
+ */
+
+// 3-row x 2-col table; each row identifiable by its cells.
+const grid2x3 = () =>
+  doc(
+    table(
+      row(cell("r0a"), cell("r0b")),
+      row(cell("r1a"), cell("r1b")),
+      row(cell("r2a"), cell("r2b")),
+    ),
+  );
+
+describe("moveRow", () => {
+  it("moves the first row down to the last index, preserving content", () => {
+    // origin 0 -> target 2 makes row 0 land after the other rows: [r1, r2, r0].
+    const state = stateFor(grid2x3());
+    const tr = state.tr;
+    const ok = moveRow({
+      tr,
+      originIndex: 0,
+      targetIndex: 2,
+      select: false,
+      pos: state.selection.from,
+    });
+    expect(ok).toBe(true);
+    expect(grid(tr)).toEqual([
+      ["r1a", "r1b"],
+      ["r2a", "r2b"],
+      ["r0a", "r0b"],
+    ]);
+  });
+
+  it("moves a lower row up to an earlier index", () => {
+    // origin 2 -> target 0 lifts the last row above the rest: [r2, r0, r1].
+    const state = stateFor(grid2x3());
+    const tr = state.tr;
+    const ok = moveRow({
+      tr,
+      originIndex: 2,
+      targetIndex: 0,
+      select: false,
+      pos: state.selection.from,
+    });
+    expect(ok).toBe(true);
+    expect(grid(tr)).toEqual([
+      ["r2a", "r2b"],
+      ["r0a", "r0b"],
+      ["r1a", "r1b"],
+    ]);
+  });
+
+  it("never drops or duplicates rows when reordering", () => {
+    // The full multiset of cell texts is invariant under any valid move.
+    const state = stateFor(grid2x3());
+    const tr = state.tr;
+    moveRow({
+      tr,
+      originIndex: 1,
+      targetIndex: 2,
+      select: false,
+      pos: state.selection.from,
+    });
+    const flat = grid(tr).flat().sort();
+    expect(flat).toEqual(
+      ["r0a", "r0b", "r1a", "r1b", "r2a", "r2b"].sort(),
+    );
+    expect(grid(tr).length).toBe(3);
+  });
+
+  it("returns false (no-op) when target equals origin", () => {
+    // Moving a row onto itself is rejected and leaves the table unchanged.
+    const state = stateFor(grid2x3());
+    const tr = state.tr;
+    const before = grid(tr);
+    const ok = moveRow({
+      tr,
+      originIndex: 1,
+      targetIndex: 1,
+      select: false,
+      pos: state.selection.from,
+    });
+    expect(ok).toBe(false);
+    expect(grid(tr)).toEqual(before);
+  });
+
+  it("returns false when pos is not inside a table", () => {
+    // Without a table at `pos`, the function bails out instead of throwing.
+    const d = doc(
+      schema.nodes.paragraph.createChecked(null, schema.text("plain")),
+    );
+    const state = stateFor(d);
+    const tr = state.tr;
+    const ok = moveRow({
+      tr,
+      originIndex: 0,
+      targetIndex: 1,
+      select: false,
+      pos: state.selection.from,
+    });
+    expect(ok).toBe(false);
+  });
+
+  it("installs a CellSelection on the moved row when select is true", () => {
+    // With select:true the moved row at the target index is selected.
+    const state = stateFor(grid2x3());
+    const tr = state.tr;
+    const ok = moveRow({
+      tr,
+      originIndex: 0,
+      targetIndex: 2,
+      select: true,
+      pos: state.selection.from,
+    });
+    expect(ok).toBe(true);
+    expect(tr.selection instanceof CellSelection).toBe(true);
+  });
+});

packages/editor-ext/src/lib/table/utils/table-test-helpers.ts

@@ -0,0 +1,60 @@
+import { Schema } from "@tiptap/pm/model";
+import type { Node as PMNode } from "@tiptap/pm/model";
+import { tableNodes } from "@tiptap/pm/tables";
+import { EditorState, Selection } from "@tiptap/pm/state";
+import { findTable } from "./query";
+import { convertTableNodeToArrayOfRows } from "./convert-table-node-to-array-of-rows";
+
+/**
+ * Shared test fixtures for the table utility tests. Several test files exercise
+ * the row/column move and selection helpers against a real ProseMirror table
+ * schema (the same primitives the editor uses) so TableMap / cellsInRect behave
+ * exactly as in production. Keeping the schema and node builders in one place
+ * means a schema change (e.g. cellAttributes) is applied once instead of being
+ * copied across every test file.
+ *
+ * This is a test-only helper (not shipped). Its name does not match vitest's
+ * `*.{test,spec}.ts` include glob, so it is not collected as a spec file —
+ * adding test cases here would NOT make them run; put them in a `*.test.ts`.
+ */
+
+const tNodes = tableNodes({
+  tableGroup: "block",
+  cellContent: "inline*",
+  cellAttributes: {},
+});
+
+export const schema = new Schema({
+  nodes: {
+    doc: { content: "block+" },
+    paragraph: { group: "block", content: "inline*", toDOM: () => ["p", 0] },
+    text: { group: "inline" },
+    ...tNodes,
+  },
+  marks: {},
+});
+
+export const cell = (txt: string, attrs?: Record<string, unknown>): PMNode =>
+  schema.nodes.table_cell.createChecked(attrs ?? null, schema.text(txt));
+export const row = (...cells: PMNode[]): PMNode =>
+  schema.nodes.table_row.createChecked(null, cells);
+export const table = (...rows: PMNode[]): PMNode =>
+  schema.nodes.table.createChecked(null, rows);
+export const doc = (...content: PMNode[]): PMNode =>
+  schema.nodes.doc.createChecked(null, content);
+
+// Read the table's content as a grid of cell texts (rows x cols) from whatever
+// table currently lives in `tr.doc`.
+export const grid = (tr: any): string[][] => {
+  const t = findTable(tr.doc.resolve(tr.selection.from))!;
+  return convertTableNodeToArrayOfRows(t.node).map((r) =>
+    r.map((c) => (c ? c.textContent : "")),
+  );
+};
+
+export const stateFor = (d: PMNode) =>
+  EditorState.create({ doc: d, selection: Selection.atStart(d) });
+
+// Build a transaction whose selection is inside the doc (helpers locate the
+// table via `tr.selection.$from`).
+export const trFor = (d: PMNode) => stateFor(d).tr;

packages/editor-ext/src/lib/table/utils/table-utils.test.ts

@@ -1,11 +1,11 @@
 import { describe, it, expect } from "vitest";
-import { Schema } from "@tiptap/pm/model";
 import type { Node as PMNode } from "@tiptap/pm/model";
-import { tableNodes, TableMap } from "@tiptap/pm/tables";
+import { TableMap } from "@tiptap/pm/tables";
 import { transpose } from "./transpose";
 import { moveRowInArrayOfRows } from "./move-row-in-array-of-rows";
 import { convertTableNodeToArrayOfRows } from "./convert-table-node-to-array-of-rows";
 import { convertArrayOfRowsToTableNode } from "./convert-array-of-rows-to-table-node";
+import { cell, row, table } from "./table-test-helpers";
 
 /**
  * Unit tests for the pure table data-transformation utilities. These functions
@@ -14,30 +14,6 @@ import { convertArrayOfRowsToTableNode } from "./convert-array-of-rows-to-table-
  * ProseMirror table schema (the same primitives the editor uses).
  */
 
-// Minimal schema containing real ProseMirror table nodes so TableMap behaves
-// exactly as it does in the editor (merged cells, colspan, etc.).
-const tNodes = tableNodes({
-  tableGroup: "block",
-  cellContent: "inline*",
-  cellAttributes: {},
-});
-const schema = new Schema({
-  nodes: {
-    doc: { content: "block+" },
-    paragraph: { group: "block", content: "inline*", toDOM: () => ["p", 0] },
-    text: { group: "inline" },
-    ...tNodes,
-  },
-  marks: {},
-});
-
-const cell = (txt: string, attrs?: Record<string, unknown>): PMNode =>
-  schema.nodes.table_cell.createChecked(attrs ?? null, schema.text(txt));
-const row = (...cells: PMNode[]): PMNode =>
-  schema.nodes.table_row.createChecked(null, cells);
-const table = (...rows: PMNode[]): PMNode =>
-  schema.nodes.table.createChecked(null, rows);
-
 // Read the text content of each (non-null) cell so we can compare structure
 // without depending on ProseMirror node identity.
 const textGrid = (rows: (PMNode | null)[][]): (string | null)[][] =>

packages/editor-ext/src/lib/unique-id/unique-id.util.test.ts

@@ -100,4 +100,51 @@ describe("addUniqueIdsToDoc", () => {
     const [id] = ids(out);
     expect(id).toBeTruthy();
   });
+
+  it("only assigns ids to configured node types, not to others", () => {
+    // `types` is ["heading","paragraph"]; a codeBlock is NOT addressed, so it
+    // must come back without an id while the sibling paragraph is filled. (The
+    // UniqueID attribute only exists on configured types in the schema.)
+    const doc = {
+      type: "doc",
+      content: [
+        { type: "codeBlock", content: [{ type: "text", text: "x = 1" }] },
+        para(undefined, "after"),
+      ],
+    };
+    const out = addUniqueIdsToDoc(doc, extensions);
+    const [codeId, paraId] = ids(out);
+    expect(codeId).toBeUndefined();
+    expect(paraId).toBeTruthy();
+  });
+
+  it("assigns ids to target nodes nested inside non-target containers", () => {
+    // findChildren walks the whole tree: a paragraph inside a blockquote still
+    // gets an id, while the (non-target) blockquote wrapper does not.
+    const doc = {
+      type: "doc",
+      content: [
+        { type: "blockquote", content: [para(undefined, "quoted")] },
+      ],
+    };
+    const out = addUniqueIdsToDoc(doc, extensions) as any;
+    const blockquote = out.content[0];
+    const nestedPara = blockquote.content[0];
+    expect(blockquote.attrs?.id).toBeUndefined();
+    expect(nestedPara.attrs.id).toBeTruthy();
+  });
+
+  it("is idempotent: a second pass keeps every already-unique id unchanged", () => {
+    // Once ids are assigned and unique, re-running must be a fixed point — no
+    // churn that would invalidate stored MCP anchors on every save.
+    const doc = {
+      type: "doc",
+      content: [para(undefined, "a"), para(undefined, "b"), para(undefined, "c")],
+    };
+    const once = addUniqueIdsToDoc(doc, extensions);
+    const twice = addUniqueIdsToDoc(once, extensions);
+    expect(ids(twice)).toEqual(ids(once));
+    // And all three are distinct, so the second pass had real ids to preserve.
+    expect(new Set(ids(once)).size).toBe(3);
+  });
 });

packages/editor-ext/tsconfig.json

@@ -27,6 +27,7 @@
     "dist",
     "src/**/*.spec.ts",
     "src/**/*.test.ts",
-    "src/lib/footnote/footnote-corpus.ts"
+    "src/lib/footnote/footnote-corpus.ts",
+    "src/lib/table/utils/table-test-helpers.ts"
   ]
 }

packages/mcp/README.md

@@ -16,7 +16,7 @@ license.
 > that interface. Other Docmost MCPs are human-shaped — they expose "open the page" and
 > "replace the page"; this one exposes the editing primitives a model is good at.
 
-It exposes **40 tools** built around three ideas that the other Docmost MCPs do not
+It exposes **41 tools** built around three ideas that the other Docmost MCPs do not
 combine:
 
 1. **Surgical, token-cheap edits.** Address a single block by id and patch it, or run
@@ -106,7 +106,7 @@ There are several Docmost MCPs. Here is a capability-by-capability comparison.
 
 ## Tools
 
-All 40 tools, grouped by what you'd reach for them.
+All 41 tools, grouped by what you'd reach for them.
 
 ### Exploration & retrieval
 
@@ -219,6 +219,8 @@ All 40 tools, grouped by what you'd reach for them.
 - **`list_comments`** — List a page's comments (content returned as Markdown).
 - **`update_comment`** — Edit an existing comment.
 - **`delete_comment`** — Delete a comment.
+- **`resolve_comment`** — Resolve (close) or reopen a comment thread (reversible). Only top-level
+  comments can be resolved; the thread and its replies are kept, unlike `delete_comment`.
 - **`check_new_comments`** — Find comments created after a given ISO-8601 timestamp across
   a space, optionally scoped to a page subtree — ideal for an agent that watches a doc for
   feedback.
@@ -262,7 +264,7 @@ so capable clients steer the model automatically.
 - **Reads**: `get_page` (Markdown) / `get_page_json` (lossless ProseMirror with ids).
 - **Review changes**: `list_page_history` → `diff_page_versions` → `restore_page_version`.
 - **Comments**: `create_comment` (with optional inline anchoring) / `list_comments` /
-  `update_comment` / `delete_comment` / `check_new_comments`.
+  `update_comment` / `resolve_comment` / `delete_comment` / `check_new_comments`.
 - **Navigate a page cheaply** (find a section/table, grab a block id): `get_outline` →
   `get_node`.
 - **Tables** (add/remove a row, set a cell): `table_get` / `table_insert_row` /

packages/mcp/README.ru.md

@@ -17,7 +17,7 @@
 > «открыть страницу» и «заменить страницу»; этот даёт примитивы редактирования, в которых
 > модель сильна.
 
-Сервер предоставляет **40 инструментов**, построенных вокруг трёх идей, которые другие
+Сервер предоставляет **41 инструмент**, построенный вокруг трёх идей, которые другие
 Docmost-MCP не сочетают:
 
 1. **Точечные, экономичные по токенам правки.** Адресуйте отдельный блок по id и патчите
@@ -109,7 +109,7 @@ Docmost-MCP не сочетают:
 
 ## Инструменты
 
-Все 40 инструментов, сгруппированы по задачам, для которых вы их возьмёте.
+Все 41 инструмент, сгруппированы по задачам, для которых вы их возьмёте.
 
 ### Чтение и поиск
 
@@ -226,6 +226,8 @@ Docmost-MCP не сочетают:
 - **`list_comments`** — Список комментариев страницы (контент возвращается как Markdown).
 - **`update_comment`** — Изменить существующий комментарий.
 - **`delete_comment`** — Удалить комментарий.
+- **`resolve_comment`** — Закрыть (resolve) или переоткрыть тред комментария (обратимо). Resolve
+  доступен только для корневых комментариев; тред и ответы сохраняются, в отличие от `delete_comment`.
 - **`check_new_comments`** — Найти комментарии, созданные после заданной метки времени
   ISO-8601, по пространству, опционально в рамках поддерева страниц — идеально для агента,
   который следит за обратной связью в документе.
@@ -271,7 +273,7 @@ Docmost-MCP не сочетают:
 - **Просмотр изменений**: `list_page_history` → `diff_page_versions` →
   `restore_page_version`.
 - **Комментарии**: `create_comment` (с опциональной inline-привязкой) / `list_comments` /
-  `update_comment` / `delete_comment` / `check_new_comments`.
+  `update_comment` / `resolve_comment` / `delete_comment` / `check_new_comments`.
 - **Дешёвая навигация по странице** (найти раздел/таблицу, получить id блока): `get_outline`
   → `get_node`.
 - **Таблицы** (добавить/удалить строку, задать ячейку): `table_get` / `table_insert_row` /

packages/mcp/build/index.js

@@ -27,7 +27,7 @@ const VERSION = packageJson.version;
 // --- Modern McpServer Implementation ---
 // Editing guide surfaced to MCP clients in the initialize result so they can
 // pick the right tool by intent and avoid resending whole documents.
-const SERVER_INSTRUCTIONS = "Docmost editing guide — choose the tool by intent: fix wording/typos/numbers (text inside blocks) -> edit_page_text (no node id needed). Change ONE block (paragraph/heading/callout/table cell/etc.) structurally -> patch_node (address by attrs.id from get_page_json). Add a block -> insert_node (before/after a block by attrs.id or by anchor text, or append). Remove a block -> delete_node (by attrs.id). Images -> insert_image (add an image from a web URL) / replace_image (swap an existing image for one from a web URL). New page -> create_page (Markdown). Bulk/structural rewrite or nodes without an id -> update_page_json (full ProseMirror replace; prefer the granular tools above to avoid resending the whole ~100KB+ document). Copy/replace a page's whole content from another page (server-side, no document through the model) -> copy_page_content. Rename a page (title only) -> rename_page. Read -> get_page (Markdown, lossy) or get_page_json (lossless ProseMirror with block ids). Comments -> create_comment (always inline; requires an EXACT selection — the contiguous text to anchor/highlight on; fails rather than leaving an unanchored comment), list_comments, update_comment, delete_comment, check_new_comments. Tip: read block ids via get_page_json, then use patch_node/insert_node/delete_node so you never resend the full document. " +
+const SERVER_INSTRUCTIONS = "Docmost editing guide — choose the tool by intent: fix wording/typos/numbers (text inside blocks) -> edit_page_text (no node id needed). Change ONE block (paragraph/heading/callout/table cell/etc.) structurally -> patch_node (address by attrs.id from get_page_json). Add a block -> insert_node (before/after a block by attrs.id or by anchor text, or append). Remove a block -> delete_node (by attrs.id). Images -> insert_image (add an image from a web URL) / replace_image (swap an existing image for one from a web URL). New page -> create_page (Markdown). Bulk/structural rewrite or nodes without an id -> update_page_json (full ProseMirror replace; prefer the granular tools above to avoid resending the whole ~100KB+ document). Copy/replace a page's whole content from another page (server-side, no document through the model) -> copy_page_content. Rename a page (title only) -> rename_page. Read -> get_page (Markdown, lossy) or get_page_json (lossless ProseMirror with block ids). Comments -> create_comment (always inline; requires an EXACT selection — the contiguous text to anchor/highlight on; fails rather than leaving an unanchored comment), list_comments, update_comment, resolve_comment (resolve/reopen a thread, reversible — prefer over delete to close), delete_comment, check_new_comments. Tip: read block ids via get_page_json, then use patch_node/insert_node/delete_node so you never resend the full document. " +
     "Complex/scripted rewrite (multiple coordinated edits, footnotes, renumbering) -> docmost_transform: write a JS `(doc, ctx) => doc` transform, preview the diff with dryRun (default), then apply with dryRun:false; ctx.helpers includes commentsToFootnotes for turning inline comments into numbered footnotes. " +
     "Review what changed -> diff_page_versions (compare a historyId to current, or two history versions). See a page's saved versions -> list_page_history. Undo a bad edit -> restore_page_version (writes a past version back as current; itself revertible). " +
     "Lossless markdown round-trip (download, edit, re-upload, incl. comment anchors) -> export_page_markdown / import_page_markdown.";
@@ -603,6 +603,27 @@ export function createDocmostMcpServer(config) {
             ],
         };
     });
+    // Tool: resolve_comment
+    server.registerTool("resolve_comment", {
+        description: "Resolve (close) or reopen a comment thread. Only top-level comments can " +
+            "be resolved — the server rejects resolving a reply. Reversible: pass " +
+            "resolved=false to reopen. Resolving keeps the thread and its replies " +
+            "(unlike delete_comment, which permanently removes them).",
+        inputSchema: {
+            commentId: z
+                .string()
+                .min(1)
+                .describe("ID of the top-level comment thread to resolve or reopen"),
+            resolved: z
+                .boolean()
+                .optional()
+                .default(true)
+                .describe("true (default) marks the thread resolved/closed; false reopens it"),
+        },
+    }, async ({ commentId, resolved }) => {
+        const result = await docmostClient.resolveComment(commentId, resolved);
+        return jsonContent(result);
+    });
     // Tool: check_new_comments
     server.registerTool("check_new_comments", {
         description: "Check for new comments across pages in a space since a given timestamp. " +

packages/mcp/build/lib/auth-utils.js

@@ -29,6 +29,41 @@ export async function getCollabToken(baseUrl, apiToken) {
         throw error;
     }
 }
+/**
+ * Pure cookie-parsing helper extracted from `performLogin` so the parsing logic
+ * can be unit-tested without performing the login network request. Given the
+ * raw `Set-Cookie` header array from the login response, return the `authToken`
+ * cookie's value.
+ *
+ * Behavior (kept identical to the original inline logic):
+ *  - throws if there is no Set-Cookie header at all;
+ *  - matches the cookie NAME exactly (`authToken`), so a future
+ *    `authTokenRefresh=...` cookie is NOT picked up (a `startsWith` would be);
+ *  - returns everything after the FIRST `=` up to the first `;`, so a base64
+ *    value containing `=` padding is preserved (a naive `split("=")` would
+ *    truncate it);
+ *  - cookie attributes after the first `;` (Path, HttpOnly, Expires, …) are
+ *    ignored;
+ *  - throws if no `authToken` cookie is present.
+ */
+export function extractAuthTokenFromSetCookie(cookies) {
+    if (!cookies) {
+        throw new Error("No Set-Cookie header found in login response");
+    }
+    // Match the cookie name exactly to avoid matching a future
+    // authTokenRefresh cookie (startsWith would catch it).
+    const authCookie = cookies.find((c) => {
+        const kv = c.split(";")[0];
+        return kv.slice(0, kv.indexOf("=")) === "authToken";
+    });
+    if (!authCookie) {
+        throw new Error("No authToken cookie found in login response");
+    }
+    // Take everything after the FIRST "=" up to the first ";".
+    // Splitting on "=" would truncate base64 values containing "=" padding.
+    const kv = authCookie.split(";")[0];
+    return kv.slice(kv.indexOf("=") + 1);
+}
 export async function performLogin(baseUrl, email, password) {
     try {
         const response = await axios.post(`${baseUrl}/auth/login`, {
@@ -36,24 +71,7 @@ export async function performLogin(baseUrl, email, password) {
             password,
         });
         // Extract token from Set-Cookie header
-        const cookies = response.headers["set-cookie"];
-        if (!cookies) {
-            throw new Error("No Set-Cookie header found in login response");
-        }
-        // Match the cookie name exactly to avoid matching a future
-        // authTokenRefresh cookie (startsWith would catch it).
-        const authCookie = cookies.find((c) => {
-            const kv = c.split(";")[0];
-            return kv.slice(0, kv.indexOf("=")) === "authToken";
-        });
-        if (!authCookie) {
-            throw new Error("No authToken cookie found in login response");
-        }
-        // Take everything after the FIRST "=" up to the first ";".
-        // Splitting on "=" would truncate base64 values containing "=" padding.
-        const kv = authCookie.split(";")[0];
-        const token = kv.slice(kv.indexOf("=") + 1);
-        return token;
+        return extractAuthTokenFromSetCookie(response.headers["set-cookie"]);
     }
     catch (error) {
         // Avoid leaking the full server response body by default; log only the

packages/mcp/build/lib/docmost-schema.js

@@ -271,25 +271,6 @@ const TextStyle = Mark.create({
         return ["span", HTMLAttributes, 0];
     },
 });
-/**
- * Inline spoiler mark. Mirrors the @docmost/editor-ext `spoiler` mark so a
- * document carrying a spoiler survives the MCP read -> transform -> write path
- * (and markdown export) instead of silently dropping the unrecognized mark.
- * packages/mcp does NOT depend on editor-ext, so the definition is kept local;
- * it parses span[data-spoiler] and renders the same span[data-spoiler][class]
- * the editor-ext mark emits.
- */
-const Spoiler = Mark.create({
-    name: "spoiler",
-    // Don't bleed onto text typed at the boundary (mirrors editor-ext).
-    inclusive: false,
-    parseHTML() {
-        return [{ tag: "span[data-spoiler]" }];
-    },
-    renderHTML({ HTMLAttributes }) {
-        return ["span", { "data-spoiler": "true", class: "spoiler", ...HTMLAttributes }, 0];
-    },
-});
 /**
  * Passthrough definitions for the remaining Docmost-specific nodes.
  *
@@ -1116,7 +1097,6 @@ export const docmostExtensions = [
     // generateJSON drops <span style="color: ...">, defeating the color import.
     TextStyle,
     Comment,
-    Spoiler,
     Callout,
     Table,
     TableRow,

packages/mcp/build/lib/markdown-converter.js

@@ -160,12 +160,6 @@ export function convertProseMirrorToMarkdown(content) {
                                 }
                                 break;
                             }
-                            case "spoiler":
-                                // Markdown has no native spoiler syntax, so emit the same
-                                // lossless raw HTML the editor-ext turndown rule produces; the
-                                // schema's Spoiler mark parses span[data-spoiler] back on import.
-                                textContent = `<span data-spoiler="true">${textContent}</span>`;
-                                break;
                         }
                     }
                 }

packages/mcp/src/index.ts

@@ -38,7 +38,7 @@ const VERSION = packageJson.version;
 // Editing guide surfaced to MCP clients in the initialize result so they can
 // pick the right tool by intent and avoid resending whole documents.
 const SERVER_INSTRUCTIONS =
-  "Docmost editing guide — choose the tool by intent: fix wording/typos/numbers (text inside blocks) -> edit_page_text (no node id needed). Change ONE block (paragraph/heading/callout/table cell/etc.) structurally -> patch_node (address by attrs.id from get_page_json). Add a block -> insert_node (before/after a block by attrs.id or by anchor text, or append). Remove a block -> delete_node (by attrs.id). Images -> insert_image (add an image from a web URL) / replace_image (swap an existing image for one from a web URL). New page -> create_page (Markdown). Bulk/structural rewrite or nodes without an id -> update_page_json (full ProseMirror replace; prefer the granular tools above to avoid resending the whole ~100KB+ document). Copy/replace a page's whole content from another page (server-side, no document through the model) -> copy_page_content. Rename a page (title only) -> rename_page. Read -> get_page (Markdown, lossy) or get_page_json (lossless ProseMirror with block ids). Comments -> create_comment (always inline; requires an EXACT selection — the contiguous text to anchor/highlight on; fails rather than leaving an unanchored comment), list_comments, update_comment, delete_comment, check_new_comments. Tip: read block ids via get_page_json, then use patch_node/insert_node/delete_node so you never resend the full document. " +
+  "Docmost editing guide — choose the tool by intent: fix wording/typos/numbers (text inside blocks) -> edit_page_text (no node id needed). Change ONE block (paragraph/heading/callout/table cell/etc.) structurally -> patch_node (address by attrs.id from get_page_json). Add a block -> insert_node (before/after a block by attrs.id or by anchor text, or append). Remove a block -> delete_node (by attrs.id). Images -> insert_image (add an image from a web URL) / replace_image (swap an existing image for one from a web URL). New page -> create_page (Markdown). Bulk/structural rewrite or nodes without an id -> update_page_json (full ProseMirror replace; prefer the granular tools above to avoid resending the whole ~100KB+ document). Copy/replace a page's whole content from another page (server-side, no document through the model) -> copy_page_content. Rename a page (title only) -> rename_page. Read -> get_page (Markdown, lossy) or get_page_json (lossless ProseMirror with block ids). Comments -> create_comment (always inline; requires an EXACT selection — the contiguous text to anchor/highlight on; fails rather than leaving an unanchored comment), list_comments, update_comment, resolve_comment (resolve/reopen a thread, reversible — prefer over delete to close), delete_comment, check_new_comments. Tip: read block ids via get_page_json, then use patch_node/insert_node/delete_node so you never resend the full document. " +
   "Complex/scripted rewrite (multiple coordinated edits, footnotes, renumbering) -> docmost_transform: write a JS `(doc, ctx) => doc` transform, preview the diff with dryRun (default), then apply with dryRun:false; ctx.helpers includes commentsToFootnotes for turning inline comments into numbered footnotes. " +
   "Review what changed -> diff_page_versions (compare a historyId to current, or two history versions). See a page's saved versions -> list_page_history. Undo a bad edit -> restore_page_version (writes a past version back as current; itself revertible). " +
   "Lossless markdown round-trip (download, edit, re-upload, incl. comment anchors) -> export_page_markdown / import_page_markdown.";
@@ -838,6 +838,35 @@ server.registerTool(
   },
 );
 
+// Tool: resolve_comment
+server.registerTool(
+  "resolve_comment",
+  {
+    description:
+      "Resolve (close) or reopen a comment thread. Only top-level comments can " +
+      "be resolved — the server rejects resolving a reply. Reversible: pass " +
+      "resolved=false to reopen. Resolving keeps the thread and its replies " +
+      "(unlike delete_comment, which permanently removes them).",
+    inputSchema: {
+      commentId: z
+        .string()
+        .min(1)
+        .describe("ID of the top-level comment thread to resolve or reopen"),
+      resolved: z
+        .boolean()
+        .optional()
+        .default(true)
+        .describe(
+          "true (default) marks the thread resolved/closed; false reopens it",
+        ),
+    },
+  },
+  async ({ commentId, resolved }) => {
+    const result = await docmostClient.resolveComment(commentId, resolved);
+    return jsonContent(result);
+  },
+);
+
 // Tool: check_new_comments
 server.registerTool(
   "check_new_comments",

packages/mcp/src/lib/auth-utils.ts

@@ -38,6 +38,45 @@ export async function getCollabToken(
   }
 }
 
+/**
+ * Pure cookie-parsing helper extracted from `performLogin` so the parsing logic
+ * can be unit-tested without performing the login network request. Given the
+ * raw `Set-Cookie` header array from the login response, return the `authToken`
+ * cookie's value.
+ *
+ * Behavior (kept identical to the original inline logic):
+ *  - throws if there is no Set-Cookie header at all;
+ *  - matches the cookie NAME exactly (`authToken`), so a future
+ *    `authTokenRefresh=...` cookie is NOT picked up (a `startsWith` would be);
+ *  - returns everything after the FIRST `=` up to the first `;`, so a base64
+ *    value containing `=` padding is preserved (a naive `split("=")` would
+ *    truncate it);
+ *  - cookie attributes after the first `;` (Path, HttpOnly, Expires, …) are
+ *    ignored;
+ *  - throws if no `authToken` cookie is present.
+ */
+export function extractAuthTokenFromSetCookie(
+  cookies: string[] | undefined,
+): string {
+  if (!cookies) {
+    throw new Error("No Set-Cookie header found in login response");
+  }
+  // Match the cookie name exactly to avoid matching a future
+  // authTokenRefresh cookie (startsWith would catch it).
+  const authCookie = cookies.find((c: string) => {
+    const kv = c.split(";")[0];
+    return kv.slice(0, kv.indexOf("=")) === "authToken";
+  });
+  if (!authCookie) {
+    throw new Error("No authToken cookie found in login response");
+  }
+
+  // Take everything after the FIRST "=" up to the first ";".
+  // Splitting on "=" would truncate base64 values containing "=" padding.
+  const kv = authCookie.split(";")[0];
+  return kv.slice(kv.indexOf("=") + 1);
+}
+
 export async function performLogin(
   baseUrl: string,
   email: string,
@@ -50,25 +89,7 @@ export async function performLogin(
     });
 
     // Extract token from Set-Cookie header
-    const cookies = response.headers["set-cookie"];
-    if (!cookies) {
-      throw new Error("No Set-Cookie header found in login response");
-    }
-    // Match the cookie name exactly to avoid matching a future
-    // authTokenRefresh cookie (startsWith would catch it).
-    const authCookie = cookies.find((c: string) => {
-      const kv = c.split(";")[0];
-      return kv.slice(0, kv.indexOf("=")) === "authToken";
-    });
-    if (!authCookie) {
-      throw new Error("No authToken cookie found in login response");
-    }
-
-    // Take everything after the FIRST "=" up to the first ";".
-    // Splitting on "=" would truncate base64 values containing "=" padding.
-    const kv = authCookie.split(";")[0];
-    const token = kv.slice(kv.indexOf("=") + 1);
-    return token;
+    return extractAuthTokenFromSetCookie(response.headers["set-cookie"]);
   } catch (error: any) {
     // Avoid leaking the full server response body by default; log only the
     // HTTP status. Log the verbose body only when DEBUG is set.

packages/mcp/src/lib/docmost-schema.ts

@@ -298,26 +298,6 @@ const TextStyle = Mark.create({
   },
 });
 
-/**
- * Inline spoiler mark. Mirrors the @docmost/editor-ext `spoiler` mark so a
- * document carrying a spoiler survives the MCP read -> transform -> write path
- * (and markdown export) instead of silently dropping the unrecognized mark.
- * packages/mcp does NOT depend on editor-ext, so the definition is kept local;
- * it parses span[data-spoiler] and renders the same span[data-spoiler][class]
- * the editor-ext mark emits.
- */
-const Spoiler = Mark.create({
-  name: "spoiler",
-  // Don't bleed onto text typed at the boundary (mirrors editor-ext).
-  inclusive: false,
-  parseHTML() {
-    return [{ tag: "span[data-spoiler]" }];
-  },
-  renderHTML({ HTMLAttributes }) {
-    return ["span", { "data-spoiler": "true", class: "spoiler", ...HTMLAttributes }, 0];
-  },
-});
-
 /**
  * Passthrough definitions for the remaining Docmost-specific nodes.
  *
@@ -1214,7 +1194,6 @@ export const docmostExtensions = [
   // generateJSON drops <span style="color: ...">, defeating the color import.
   TextStyle,
   Comment,
-  Spoiler,
   Callout,
   Table,
   TableRow,

packages/mcp/src/lib/markdown-converter.ts

@@ -167,12 +167,6 @@ export function convertProseMirrorToMarkdown(content: any): string {
                 }
                 break;
               }
-              case "spoiler":
-                // Markdown has no native spoiler syntax, so emit the same
-                // lossless raw HTML the editor-ext turndown rule produces; the
-                // schema's Spoiler mark parses span[data-spoiler] back on import.
-                textContent = `<span data-spoiler="true">${textContent}</span>`;
-                break;
             }
           }
         }

packages/mcp/test-e2e.mjs

@@ -469,6 +469,17 @@ async function main() {
     check("update_comment + get_comment: content updated", got.data.content.includes("Обновлённый"), got.data.content);
     const news = await client.checkNewComments(spaceId, beforeComments, pageId);
     check("check_new_comments: finds new comments in subtree", news.totalNewComments >= 2, `total=${news.totalNewComments}`);
+    // resolve_comment: close the top-level thread, verify resolvedAt surfaces, then reopen
+    const resolvedRes = await client.resolveComment(c1.data.id, true);
+    check("resolve_comment: marks resolved", resolvedRes.success === true && resolvedRes.resolved === true);
+    const listResolved = await client.listComments(pageId);
+    const c1Resolved = listResolved.find((c) => c.id === c1.data.id);
+    check("resolve_comment: resolvedAt set in list", !!c1Resolved?.resolvedAt, `resolvedAt=${c1Resolved?.resolvedAt}`);
+    const reopenedRes = await client.resolveComment(c1.data.id, false);
+    check("resolve_comment: reopen succeeds", reopenedRes.resolved === false);
+    const listReopened = await client.listComments(pageId);
+    const c1Reopened = listReopened.find((c) => c.id === c1.data.id);
+    check("resolve_comment: resolvedAt cleared on reopen", !c1Reopened?.resolvedAt, `resolvedAt=${c1Reopened?.resolvedAt}`);
     await client.deleteComment(reply.data.id);
     await client.deleteComment(c1.data.id);
     const listAfter = await client.listComments(pageId);

packages/mcp/test/unit/auth-cookie.test.mjs

@@ -0,0 +1,93 @@
+// Cookie parsing for the login flow.
+//
+// `performLogin` in auth-utils.ts does a real network POST and then extracts the
+// auth token from the response's Set-Cookie header. The cookie-parsing logic was
+// extracted into the pure, exported helper `extractAuthTokenFromSetCookie` so it
+// can be tested without network I/O; `performLogin` now delegates to it, so these
+// tests cover the exact parsing path the login uses.
+import { test } from "node:test";
+import assert from "node:assert/strict";
+
+import { extractAuthTokenFromSetCookie } from "../../build/lib/auth-utils.js";
+
+// ---------------------------------------------------------------------------
+// Happy path: a single authToken cookie with attributes.
+// ---------------------------------------------------------------------------
+test("extracts the authToken value, ignoring trailing attributes", () => {
+  const cookies = [
+    "authToken=abc123; Path=/; HttpOnly; Secure; SameSite=Lax",
+  ];
+  assert.equal(extractAuthTokenFromSetCookie(cookies), "abc123");
+});
+
+// ---------------------------------------------------------------------------
+// A base64/JWT value containing "=" padding must NOT be truncated: only the
+// FIRST "=" separates name from value.
+// ---------------------------------------------------------------------------
+test("preserves an '=' inside the value (base64 padding is not truncated)", () => {
+  const jwt = "eyJhbGciOiJIUzI1NiJ9.eyJzdWIiOiIxIn0=";
+  const cookies = [`authToken=${jwt}; Path=/`];
+  assert.equal(extractAuthTokenFromSetCookie(cookies), jwt);
+});
+
+// ---------------------------------------------------------------------------
+// Exact-name match: a different cookie whose name merely STARTS WITH "authToken"
+// (e.g. authTokenRefresh) must not be picked up; the real authToken wins.
+// ---------------------------------------------------------------------------
+test("matches the cookie name exactly, not by prefix (authTokenRefresh ignored)", () => {
+  const cookies = [
+    "authTokenRefresh=refreshvalue; Path=/; HttpOnly",
+    "authToken=realtoken; Path=/; HttpOnly",
+  ];
+  assert.equal(extractAuthTokenFromSetCookie(cookies), "realtoken");
+});
+
+// ---------------------------------------------------------------------------
+// Picks the authToken out of several unrelated cookies regardless of order.
+// ---------------------------------------------------------------------------
+test("selects authToken among multiple unrelated cookies", () => {
+  const cookies = [
+    "session=xyz; Path=/",
+    "authToken=tok-7; Path=/; HttpOnly",
+    "theme=dark",
+  ];
+  assert.equal(extractAuthTokenFromSetCookie(cookies), "tok-7");
+});
+
+// ---------------------------------------------------------------------------
+// An empty value is valid and returns "".
+// ---------------------------------------------------------------------------
+test("returns an empty string when authToken has an empty value", () => {
+  assert.equal(extractAuthTokenFromSetCookie(["authToken=; Path=/"]), "");
+});
+
+// ---------------------------------------------------------------------------
+// Missing Set-Cookie header -> documented error.
+// ---------------------------------------------------------------------------
+test("throws when there is no Set-Cookie header", () => {
+  assert.throws(
+    () => extractAuthTokenFromSetCookie(undefined),
+    /No Set-Cookie header/,
+  );
+});
+
+// ---------------------------------------------------------------------------
+// Set-Cookie present but no authToken cookie -> documented error.
+// ---------------------------------------------------------------------------
+test("throws when no authToken cookie is present", () => {
+  assert.throws(
+    () => extractAuthTokenFromSetCookie(["session=xyz; Path=/", "theme=dark"]),
+    /No authToken cookie/,
+  );
+});
+
+// ---------------------------------------------------------------------------
+// An empty cookie array also yields the "no authToken" error (header exists but
+// is empty), distinct from the "no Set-Cookie header" case above.
+// ---------------------------------------------------------------------------
+test("throws 'no authToken' (not 'no header') for an empty cookie array", () => {
+  assert.throws(
+    () => extractAuthTokenFromSetCookie([]),
+    /No authToken cookie/,
+  );
+});

packages/mcp/test/unit/comment-anchor-apply.test.mjs

@@ -0,0 +1,111 @@
+// applyAnchorInDoc — first-match / ambiguity / boundary behavior.
+//
+// comment-anchor.test.mjs already covers the core apply paths (single-node
+// match, spanning adjacent text nodes, code/italic boundary mark preservation,
+// smart-quote normalization, no-match-no-mutation, pre-existing comment mark
+// replacement, nested-list DFS). This file focuses on the SELECTION/RESOLUTION
+// behavior those tests don't pin down: which occurrence/block wins when a
+// selection appears more than once, sub-word ranges, and the run boundary
+// created by a non-text inline node.
+import { test } from "node:test";
+import assert from "node:assert/strict";
+
+import { applyAnchorInDoc, canAnchorInDoc } from "../../build/lib/comment-anchor.js";
+
+const commentMark = (node) =>
+  (Array.isArray(node.marks) ? node.marks : []).find((m) => m && m.type === "comment") || null;
+const paragraphDoc = (content) => ({ type: "doc", content: [{ type: "paragraph", content }] });
+
+// ---------------------------------------------------------------------------
+// Document order: when two separate blocks both contain the selection, only the
+// FIRST block (DFS document order) is anchored; the second is left untouched.
+// ---------------------------------------------------------------------------
+test("anchors only the FIRST block when the selection occurs in two blocks", () => {
+  const doc = {
+    type: "doc",
+    content: [
+      { type: "paragraph", content: [{ type: "text", text: "first target here" }] },
+      { type: "paragraph", content: [{ type: "text", text: "second target here" }] },
+    ],
+  };
+  assert.equal(applyAnchorInDoc(doc, "target", "C"), true);
+
+  const marked0 = doc.content[0].content.filter((p) => commentMark(p));
+  const marked1 = doc.content[1].content.filter((p) => commentMark(p));
+  assert.equal(marked0.length, 1, "first block is anchored");
+  assert.equal(marked0[0].text, "target");
+  assert.equal(marked1.length, 0, "second block is left untouched");
+});
+
+// ---------------------------------------------------------------------------
+// Ambiguity within one block: indexOf finds the FIRST occurrence, so only the
+// first "ab" is marked; the later occurrences stay in one unmarked fragment.
+// ---------------------------------------------------------------------------
+test("anchors only the FIRST occurrence within a block (ambiguous selection)", () => {
+  const doc = paragraphDoc([{ type: "text", text: "ab ab ab" }]);
+  assert.equal(applyAnchorInDoc(doc, "ab", "C"), true);
+
+  const parts = doc.content[0].content;
+  assert.equal(parts.length, 2, "split into [marked, rest]");
+  assert.equal(parts[0].text, "ab");
+  assert.ok(commentMark(parts[0]), "first occurrence is marked");
+  assert.equal(parts[1].text, " ab ab");
+  assert.equal(commentMark(parts[1]), null, "later occurrences are not marked");
+});
+
+// ---------------------------------------------------------------------------
+// Sub-word range: a selection that is a substring inside a single text node is
+// spliced into before / marked / after, marking exactly the matched characters.
+// ---------------------------------------------------------------------------
+test("anchors a sub-word range inside a single text node", () => {
+  const doc = paragraphDoc([{ type: "text", text: "Hello" }]);
+  assert.equal(applyAnchorInDoc(doc, "ell", "C"), true);
+
+  const parts = doc.content[0].content;
+  assert.deepEqual(parts.map((p) => p.text), ["H", "ell", "o"]);
+  assert.equal(commentMark(parts[0]), null);
+  assert.ok(commentMark(parts[1]), "only the matched substring is marked");
+  assert.equal(commentMark(parts[2]), null);
+});
+
+// ---------------------------------------------------------------------------
+// A non-text inline node (hardBreak) breaks the matching run: a selection that
+// would span the break cannot match, but one wholly inside a run still does.
+// ---------------------------------------------------------------------------
+test("a non-text inline node breaks the run: cross-break selection does not match", () => {
+  const make = () =>
+    paragraphDoc([
+      { type: "text", text: "foo" },
+      { type: "hardBreak" },
+      { type: "text", text: "bar" },
+    ]);
+
+  // "foobar" straddles the hardBreak -> no match, no mutation.
+  const docA = make();
+  const before = JSON.stringify(docA);
+  assert.equal(canAnchorInDoc(docA, "foobar"), false);
+  assert.equal(applyAnchorInDoc(docA, "foobar", "C"), false);
+  assert.equal(JSON.stringify(docA), before, "failed match must not mutate");
+
+  // "foo" lives entirely in the first run -> matches and is marked; the
+  // hardBreak node is preserved untouched.
+  const docB = make();
+  assert.equal(applyAnchorInDoc(docB, "foo", "C"), true);
+  const parts = docB.content[0].content;
+  assert.equal(parts[0].text, "foo");
+  assert.ok(commentMark(parts[0]));
+  assert.equal(parts[1].type, "hardBreak", "the inline atom is preserved");
+  assert.equal(parts[2].text, "bar");
+  assert.equal(commentMark(parts[2]), null);
+});
+
+// ---------------------------------------------------------------------------
+// A whitespace-only selection normalizes to empty and never anchors.
+// ---------------------------------------------------------------------------
+test("a whitespace-only selection does not anchor and does not mutate", () => {
+  const doc = paragraphDoc([{ type: "text", text: "hello world" }]);
+  const before = JSON.stringify(doc);
+  assert.equal(canAnchorInDoc(doc, "   "), false);
+  assert.equal(applyAnchorInDoc(doc, "   ", "C"), false);
+  assert.equal(JSON.stringify(doc), before);
+});

packages/mcp/test/unit/docmost-md-roundtrip.test.mjs

@@ -167,38 +167,6 @@ test("export emits comment anchors and they round-trip back to a comment mark",
   });
 });
 
-test("export emits a spoiler span and it round-trips back to a spoiler mark", () => {
-  // A small ProseMirror doc with a text run carrying a `spoiler` mark. The MCP
-  // schema mirrors the editor-ext mark, so a spoiler must survive json -> md ->
-  // json instead of being silently dropped as an unrecognized mark.
-  const doc = {
-    type: "doc",
-    content: [
-      {
-        type: "paragraph",
-        content: [
-          { type: "text", text: "plot: " },
-          {
-            type: "text",
-            text: "the butler did it",
-            marks: [{ type: "spoiler" }],
-          },
-          { type: "text", text: " end" },
-        ],
-      },
-    ],
-  };
-
-  const body = convertProseMirrorToMarkdown(doc);
-  assert.match(body, /<span data-spoiler="true">the butler did it<\/span>/);
-
-  return markdownToProseMirror(body).then((rebuilt) => {
-    const spoilered = findTextWithMark(rebuilt, "spoiler");
-    assert.ok(spoilered, "expected a text node with a spoiler mark");
-    assert.equal(spoilered.text, "the butler did it");
-  });
-});
-
 test("drawio round-trips through export and import", () => {
   const doc = {
     type: "doc",

packages/mcp/test/unit/media-roundtrip-attrs.test.mjs

@@ -0,0 +1,135 @@
+// Extra media round-trip coverage (issue #244), complementing
+// media-roundtrip.test.mjs.
+//
+// The existing media-roundtrip.test.mjs already asserts that video, youtube,
+// embed, excalidraw, audio and pdf SURVIVE a PM -> markdown -> PM round-trip and
+// keeps their identifying src / provider / name / attachmentId. It does NOT,
+// however, exercise:
+//   * the `drawio` node (a distinct schema node that shares the excalidraw
+//     converter case) — not covered at all;
+//   * the dimension / layout attributes (width, height, align) that ride in
+//     data-* attributes — exactly where a converter<->schema mismatch silently
+//     drops a value while the node itself survives;
+//   * attribute escaping for a src containing `"` (escapeAttr) — a malformed
+//     value here would either break the round-trip or inject HTML.
+//
+// These are the gaps this file locks down.
+import { test } from "node:test";
+import assert from "node:assert/strict";
+
+import { convertProseMirrorToMarkdown } from "../../build/lib/markdown-converter.js";
+import { markdownToProseMirror } from "../../build/lib/collaboration.js";
+
+const doc = (...content) => ({ type: "doc", content });
+
+const findAll = (node, type, acc = []) => {
+  if (!node || typeof node !== "object") return acc;
+  if (node.type === type) acc.push(node);
+  for (const c of node.content || []) findAll(c, type, acc);
+  return acc;
+};
+
+// PM node -> markdown -> PM; return both the markdown and the matching nodes.
+const roundtrip = async (node, type) => {
+  const md = convertProseMirrorToMarkdown(doc(node));
+  const pm = await markdownToProseMirror(md);
+  return { md, found: findAll(pm, type) };
+};
+
+// ---------------------------------------------------------------------------
+// drawio: a separate schema node sharing the excalidraw converter case. Not
+// covered by the existing file at all, so guard its full round-trip here.
+// ---------------------------------------------------------------------------
+test("round-trip: drawio diagram survives with src, title, dimensions, align, attachmentId", async () => {
+  const { md, found } = await roundtrip(
+    {
+      type: "drawio",
+      attrs: {
+        src: "/api/files/d.drawio",
+        title: "Flow",
+        width: 400,
+        height: 300,
+        align: "left",
+        attachmentId: "dz1",
+      },
+    },
+    "drawio",
+  );
+  // The converter must emit the schema-matching div[data-type="drawio"].
+  assert.match(md, /data-type="drawio"/);
+  assert.equal(found.length, 1, "drawio node must survive the round-trip");
+  const a = found[0].attrs;
+  assert.equal(a.src, "/api/files/d.drawio");
+  assert.equal(a.title, "Flow");
+  assert.equal(a.attachmentId, "dz1");
+  assert.equal(a.align, "left");
+  // Numeric dimensions come back as strings via the schema parseHTML.
+  assert.equal(String(a.width), "400");
+  assert.equal(String(a.height), "300");
+});
+
+// ---------------------------------------------------------------------------
+// Dimension + align attrs ride in data-* (or width/height) attributes. The
+// existing file checks only src/provider/name/attachmentId, so a dropped
+// width/height/align would pass there but fail here.
+// ---------------------------------------------------------------------------
+test("round-trip: youtube preserves width/height/align (data-* attrs)", async () => {
+  const { found } = await roundtrip(
+    { type: "youtube", attrs: { src: "https://youtube.com/watch?v=x", width: 560, height: 315, align: "left" } },
+    "youtube",
+  );
+  assert.equal(found.length, 1);
+  const a = found[0].attrs;
+  assert.equal(String(a.width), "560");
+  assert.equal(String(a.height), "315");
+  assert.equal(a.align, "left");
+});
+
+test("round-trip: embed preserves provider, width/height and align", async () => {
+  const { found } = await roundtrip(
+    { type: "embed", attrs: { src: "https://e.com/x", provider: "iframe", width: 600, height: 480, align: "right" } },
+    "embed",
+  );
+  assert.equal(found.length, 1);
+  const a = found[0].attrs;
+  assert.equal(a.provider, "iframe");
+  assert.equal(String(a.width), "600");
+  assert.equal(String(a.height), "480");
+  assert.equal(a.align, "right");
+});
+
+test("round-trip: video preserves width/height and align (data-align)", async () => {
+  const { found } = await roundtrip(
+    { type: "video", attrs: { src: "/api/files/v.mp4", attachmentId: "att1", width: 640, height: 360, align: "right" } },
+    "video",
+  );
+  assert.equal(found.length, 1);
+  const a = found[0].attrs;
+  assert.equal(String(a.width), "640");
+  assert.equal(String(a.height), "360");
+  assert.equal(a.align, "right");
+});
+
+test("round-trip: pdf preserves width/height (standard attrs) plus name", async () => {
+  const { found } = await roundtrip(
+    { type: "pdf", attrs: { src: "/api/files/x.pdf", name: "x.pdf", attachmentId: "a4", width: 700, height: 900 } },
+    "pdf",
+  );
+  assert.equal(found.length, 1);
+  const a = found[0].attrs;
+  assert.equal(a.name, "x.pdf");
+  assert.equal(String(a.width), "700");
+  assert.equal(String(a.height), "900");
+});
+
+// ---------------------------------------------------------------------------
+// Escaping: a src containing a double quote must survive the attribute-quoted
+// HTML emission (escapeAttr) and re-parse to the exact original value, with no
+// node loss and no HTML injection.
+// ---------------------------------------------------------------------------
+test("round-trip: a src containing a double quote is escaped and recovered intact", async () => {
+  const tricky = 'https://e.com/x?a="b"&c=1';
+  const { found } = await roundtrip({ type: "youtube", attrs: { src: tricky } }, "youtube");
+  assert.equal(found.length, 1, "node must survive a quote-bearing src");
+  assert.equal(found[0].attrs.src, tricky, "the exact src is recovered");
+});

packages/mcp/test/unit/recreate-transform-drift.test.mjs

@@ -0,0 +1,139 @@
+// CONTRACT / DRIFT GUARD: mcp diff vs the vendored editor-ext recreate-transform.
+//
+// packages/mcp/src/lib/diff.ts computes its document diff with
+// `recreateTransform` from the published @fellow/prosemirror-recreate-transform
+// package. Docmost's in-app history editor computes the SAME diff with its own
+// vendored copy at
+// packages/editor-ext/src/lib/recreate-transform/recreateTransform.ts.
+// diff.ts's header comment claims the two are "identical" — if they ever drift,
+// the headless mcp diff would stop matching what a user sees in the app.
+//
+// This test guards that claim two ways, on representative doc pairs, using the
+// EXACT options diff.ts passes (complexSteps:false, wordDiffs:true,
+// simplifyDiff:true):
+//   1. invariant: each implementation's transform reproduces the target doc
+//      (apply(diff) == target);
+//   2. cross-copy parity: both implementations emit the SAME step sequence, so a
+//      behavioral divergence between the two copies fails this test.
+//
+// The vendored copy is TypeScript, so it is transpiled to CommonJS at test time
+// and required directly — the test runs the ACTUAL vendored source, not a stand-in.
+import { test, before } from "node:test";
+import assert from "node:assert/strict";
+import ts from "typescript";
+import fs from "node:fs";
+import path from "node:path";
+import { createRequire } from "node:module";
+import { fileURLToPath } from "node:url";
+
+import { recreateTransform as fellowRecreate } from "@fellow/prosemirror-recreate-transform";
+import { Node } from "@tiptap/pm/model";
+import { docmostSchema } from "../../build/lib/docmost-schema.js";
+
+const require = createRequire(import.meta.url);
+const HERE = path.dirname(fileURLToPath(import.meta.url));
+// .../packages/mcp/test/unit -> repo packages root.
+const PACKAGES = path.resolve(HERE, "..", "..", "..");
+const VENDOR_SRC = path.join(
+  PACKAGES,
+  "editor-ext",
+  "src",
+  "lib",
+  "recreate-transform",
+);
+// Emit transpiled CJS under mcp/build so Node resolves the hoisted deps
+// (@tiptap/pm, rfc6902, diff) up the directory tree exactly as diff.js does.
+const VENDOR_OUT = path.resolve(HERE, "..", "..", "build", "_vendored_editor_ext");
+
+// The exact options the mcp diff pipeline uses (diff.ts).
+const DIFF_OPTS = { complexSteps: false, wordDiffs: true, simplifyDiff: true };
+
+let vendoredRecreate;
+
+before(() => {
+  assert.ok(
+    fs.existsSync(VENDOR_SRC),
+    `vendored recreate-transform sources missing at ${VENDOR_SRC}`,
+  );
+  fs.rmSync(VENDOR_OUT, { recursive: true, force: true });
+  fs.mkdirSync(VENDOR_OUT, { recursive: true });
+  // Mark the output as CommonJS so relative `require("./x")` resolves to x.js.
+  fs.writeFileSync(
+    path.join(VENDOR_OUT, "package.json"),
+    JSON.stringify({ type: "commonjs" }),
+  );
+  for (const f of fs.readdirSync(VENDOR_SRC)) {
+    if (!f.endsWith(".ts")) continue;
+    const code = fs.readFileSync(path.join(VENDOR_SRC, f), "utf8");
+    const out = ts.transpileModule(code, {
+      compilerOptions: {
+        module: ts.ModuleKind.CommonJS,
+        target: ts.ScriptTarget.ES2020,
+      },
+    });
+    fs.writeFileSync(path.join(VENDOR_OUT, f.replace(/\.ts$/, ".js")), out.outputText);
+  }
+  vendoredRecreate = require(path.join(VENDOR_OUT, "index.js")).recreateTransform;
+  assert.equal(typeof vendoredRecreate, "function", "vendored recreateTransform loaded");
+});
+
+// ---------------------------------------------------------------------------
+// Builders + representative doc pairs covering the diff shapes diff.ts handles.
+// ---------------------------------------------------------------------------
+const t = (text, marks) => (marks ? { type: "text", text, marks } : { type: "text", text });
+const para = (...c) => ({ type: "paragraph", content: c });
+const doc = (...c) => ({ type: "doc", content: c });
+
+const PAIRS = [
+  // word inserted mid-sentence
+  ["insert word", doc(para(t("Hello world"))), doc(para(t("Hello brave world")))],
+  // whole block deleted
+  ["delete block", doc(para(t("keep this")), para(t("remove this"))), doc(para(t("keep this")))],
+  // word removed mid-sentence
+  ["delete word", doc(para(t("one two three"))), doc(para(t("one three")))],
+  // pure mark addition (complexSteps:false treats it as a content step)
+  ["add mark", doc(para(t("plain"))), doc(para(t("plain", [{ type: "bold" }])))],
+  // two blocks swapped (reorder)
+  ["reorder blocks", doc(para(t("a")), para(t("b"))), doc(para(t("b")), para(t("a")))],
+  // structural insert: an image node appears
+  [
+    "insert image",
+    doc(para(t("caption"))),
+    doc(para(t("caption")), { type: "image", attrs: { src: "/api/files/a.png", attachmentId: "i1" } }),
+  ],
+];
+
+const stepsJSON = (tr) => JSON.stringify(tr.steps.map((s) => s.toJSON()));
+
+for (const [label, fromJSON, toJSON] of PAIRS) {
+  test(`invariant: @fellow recreateTransform reproduces the target (${label})`, () => {
+    const from = Node.fromJSON(docmostSchema, fromJSON);
+    const to = Node.fromJSON(docmostSchema, toJSON);
+    const tr = fellowRecreate(from, to, DIFF_OPTS);
+    // apply(diff) == target, comparing schema-normalized JSON on both sides.
+    assert.equal(JSON.stringify(tr.doc.toJSON()), JSON.stringify(to.toJSON()));
+  });
+
+  test(`drift: @fellow and vendored editor-ext emit identical steps (${label})`, () => {
+    const mk = () => [
+      Node.fromJSON(docmostSchema, fromJSON),
+      Node.fromJSON(docmostSchema, toJSON),
+    ];
+    const [fA, tA] = mk();
+    const [fB, tB] = mk();
+    const trFellow = fellowRecreate(fA, tA, DIFF_OPTS);
+    const trVendor = vendoredRecreate(fB, tB, DIFF_OPTS);
+
+    // Both must reach the same target...
+    const target = JSON.stringify(tA.toJSON());
+    assert.equal(JSON.stringify(trFellow.doc.toJSON()), target, "fellow reaches target");
+    assert.equal(JSON.stringify(trVendor.doc.toJSON()), target, "vendored reaches target");
+    // ...and, critically, via the SAME step sequence. A divergence in the two
+    // recreate-transform copies' algorithm would change the steps and fail here.
+    assert.equal(
+      stepsJSON(trVendor),
+      stepsJSON(trFellow),
+      `vendored editor-ext drifted from @fellow on "${label}"`,
+    );
+  });
+}