= new Map();
constructor(
private readonly pageRepo: PageRepo,
@@ -230,6 +267,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
@@ -260,6 +310,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 || [];
@@ -404,6 +494,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;
@@ -427,6 +548,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[] {
@@ -444,6 +566,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,
diff --git a/apps/server/src/collaboration/yjs.util.spec.ts b/apps/server/src/collaboration/yjs.util.spec.ts
new file mode 100644
index 00000000..29511d6d
--- /dev/null
+++ b/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 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);
+ });
+});
diff --git a/apps/server/src/core/ai-chat/embedding/embedding-indexer.service.spec.ts b/apps/server/src/core/ai-chat/embedding/embedding-indexer.service.spec.ts
index 928702b3..38e86d12 100644
--- a/apps/server/src/core/ai-chat/embedding/embedding-indexer.service.spec.ts
+++ b/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);
+ });
+});
diff --git a/apps/server/src/core/ai-chat/embedding/embedding-indexer.service.ts b/apps/server/src/core/ai-chat/embedding/embedding-indexer.service.ts
index 5b49d92d..9d1a3a09 100644
--- a/apps/server/src/core/ai-chat/embedding/embedding-indexer.service.ts
+++ b/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 {
+ // 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). */
diff --git a/apps/server/src/core/ai-chat/external-mcp/mcp-clients.service.spec.ts b/apps/server/src/core/ai-chat/external-mcp/mcp-clients.service.spec.ts
new file mode 100644
index 00000000..53ad6191
--- /dev/null
+++ b/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 | undefined =>
+ (
+ service as unknown as {
+ decryptHeaders: (b: string | null) => Record | 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;
+
+ 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();
+ });
+});
diff --git a/apps/server/src/core/ai-chat/tools/docmost-client.loader.ts b/apps/server/src/core/ai-chat/tools/docmost-client.loader.ts
index 8221fd52..5059072d 100644
--- a/apps/server/src/core/ai-chat/tools/docmost-client.loader.ts
+++ b/apps/server/src/core/ai-chat/tools/docmost-client.loader.ts
@@ -6,6 +6,34 @@ import { esmImport } from '../../../common/helpers/esm-import';
* ESM-only `@docmost/mcp` package. We only need the constructor + the read/write
* methods used by the per-user tool adapter; the full client surface lives in
* `packages/mcp/src/client.ts`. Signatures here mirror that file exactly.
+ *
+ * DRIFT GUARD: the method NAMES below are runtime-checked against the real
+ * `DocmostClient` by `packages/mcp/test/unit/client-host-contract.test.mjs`
+ * (which can import the ESM class directly). If you rename/remove a method here
+ * or in client.ts, that test fails — so a stale mirror cannot silently ship a
+ * runtime "x is not a function" into an agent tool call. Keep the two in sync.
+ *
+ * STAGED PLAN — full derivation `DocmostClientLike = `
+ * (issue #193, layer 3) is intentionally NOT done; it stays a hand-mirror for
+ * now because of two verified blockers across the ESM(mcp)/CJS(server) boundary:
+ * 1. `@docmost/mcp` emits NO declaration files (its tsconfig has no
+ * `declaration`, package.json has no `types`/types-export) and the server
+ * tsconfig has no path mapping for it — the server only loads it via the
+ * runtime `import()` trick below, so there is no type to import today.
+ * 2. The real client methods have inferred, CONCRETE return types; the in-app
+ * tool adapter reads results through loose `Record` returns
+ * + `as` casts (e.g. `(result?.data ?? {}) as { title?: string }`).
+ * Deriving the exact type would make those casts non-overlapping ("may be a
+ * mistake") and break the build, and `Partial` test stubs
+ * would have to satisfy the full concrete surface.
+ * To do it safely later (incrementally): (a) turn on `declaration: true` in
+ * packages/mcp/tsconfig.json + add a `types` export condition and commit the
+ * emitted `.d.ts`; (b) `import type { DocmostClient } from '@docmost/mcp'` here
+ * and replace this interface with a `Pick` of the consumed
+ * methods; (c) audit every `as` cast in ai-chat-tools.service.ts against the now
+ * concrete return types (double-cast through `unknown` only where genuinely
+ * needed); (d) keep the runtime guard test as a belt-and-braces check. Until
+ * then the guard test above is the cheap, behaviour-neutral protection.
*/
export interface DocmostClientLike {
// --- read ---
diff --git a/apps/server/src/core/ai-chat/tools/shared-tool-specs.contract.spec.ts b/apps/server/src/core/ai-chat/tools/shared-tool-specs.contract.spec.ts
new file mode 100644
index 00000000..31461717
--- /dev/null
+++ b/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., 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 = {};
+ const tokenServiceStub = {
+ generateAccessToken: jest.fn().mockResolvedValue('access-token'),
+ generateCollabToken: jest.fn().mockResolvedValue('collab-token'),
+ };
+
+ let tools: Record;
+
+ 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;
+ });
+
+ 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; 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);
+ });
+ });
+});
diff --git a/apps/server/src/core/share/share-spoiler-keep.spec.ts b/apps/server/src/core/share/share-spoiler-keep.spec.ts
new file mode 100644
index 00000000..bcc68b13
--- /dev/null
+++ b/apps/server/src/core/share/share-spoiler-keep.spec.ts
@@ -0,0 +1,129 @@
+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');
+ });
+});
diff --git a/apps/server/src/database/repos/page/page.repo.embeddable.spec.ts b/apps/server/src/database/repos/page/page.repo.embeddable.spec.ts
new file mode 100644
index 00000000..1fb1828f
--- /dev/null
+++ b/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({
+ 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) => 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);
+ });
+});
diff --git a/apps/server/src/database/repos/page/page.repo.ts b/apps/server/src/database/repos/page/page.repo.ts
index 3ce207c9..50214248 100644
--- a/apps/server/src/database/repos/page/page.repo.ts
+++ b/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 {
@@ -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`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 {
+ private embeddablePredicate(
+ eb: ExpressionBuilder,
+ ) {
+ 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`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`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 {
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);
}
diff --git a/apps/server/src/integrations/ai/ai-settings.service.spec.ts b/apps/server/src/integrations/ai/ai-settings.service.spec.ts
index b0efaa21..ba263a3e 100644
--- a/apps/server/src/integrations/ai/ai-settings.service.spec.ts
+++ b/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();
+ });
+});
diff --git a/apps/server/src/integrations/ai/ai-settings.service.ts b/apps/server/src/integrations/ai/ai-settings.service.ts
index 2ccf5580..6be0b5be 100644
--- a/apps/server/src/integrations/ai/ai-settings.service.ts
+++ b/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,
};
}
diff --git a/apps/server/src/integrations/ai/ai.module.ts b/apps/server/src/integrations/ai/ai.module.ts
index 6d0ec3e9..a38c7f04 100644
--- a/apps/server/src/integrations/ai/ai.module.ts
+++ b/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 {}
diff --git a/apps/server/src/integrations/ai/ai.types.ts b/apps/server/src/integrations/ai/ai.types.ts
index efad9857..06bf83e3 100644
--- a/apps/server/src/integrations/ai/ai.types.ts
+++ b/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;
}
diff --git a/apps/server/src/integrations/ai/embedding-reindex-progress.service.spec.ts b/apps/server/src/integrations/ai/embedding-reindex-progress.service.spec.ts
new file mode 100644
index 00000000..496d55e8
--- /dev/null
+++ b/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 } = {}) {
+ 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();
+ });
+ });
+});
diff --git a/apps/server/src/integrations/ai/embedding-reindex-progress.service.ts b/apps/server/src/integrations/ai/embedding-reindex-progress.service.ts
new file mode 100644
index 00000000..2f551f0d
--- /dev/null
+++ b/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 {
+ 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 {
+ 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 {
+ 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 {
+ 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;
+ }
+ }
+}
diff --git a/apps/server/src/integrations/storage/storage.service.spec.ts b/apps/server/src/integrations/storage/storage.service.spec.ts
index 79db48c0..fc80246e 100644
--- a/apps/server/src/integrations/storage/storage.service.spec.ts
+++ b/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 {
+ 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;
+ }
+
+ let driver: jest.Mocked;
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);
});
});
diff --git a/apps/server/src/integrations/throttle/throttle.module.ts b/apps/server/src/integrations/throttle/throttle.module.ts
index 1cb0c41a..db197015 100644
--- a/apps/server/src/integrations/throttle/throttle.module.ts
+++ b/apps/server/src/integrations/throttle/throttle.module.ts
@@ -10,7 +10,6 @@ import {
PAGE_TEMPLATE_THROTTLER,
PUBLIC_SHARE_AI_THROTTLER,
} from './throttler-names';
-import Redis from 'ioredis';
@Module({
imports: [
@@ -32,16 +31,18 @@ import Redis from 'ioredis';
{ name: PUBLIC_SHARE_AI_THROTTLER, ttl: 60_000, limit: 5 },
],
errorMessage: 'Too many requests',
- storage: new ThrottlerStorageRedisService(
- new Redis({
- host: redisConfig.host,
- port: redisConfig.port,
- password: redisConfig.password,
- db: redisConfig.db,
- family: redisConfig.family,
- keyPrefix: 'throttle:',
- }),
- ),
+ // Pass ioredis options (not a pre-built Redis instance) so
+ // ThrottlerStorageRedisService owns the connection and disconnects it
+ // in its onModuleDestroy. Passing an instance leaves disconnectRequired
+ // false, so the socket would leak on shutdown (e2e jest never exits).
+ storage: new ThrottlerStorageRedisService({
+ host: redisConfig.host,
+ port: redisConfig.port,
+ password: redisConfig.password,
+ db: redisConfig.db,
+ family: redisConfig.family,
+ keyPrefix: 'throttle:',
+ }),
};
},
inject: [EnvironmentService],
diff --git a/apps/server/src/ws/adapter/ws-redis.adapter.ts b/apps/server/src/ws/adapter/ws-redis.adapter.ts
index a221c84a..4cc1c3e5 100644
--- a/apps/server/src/ws/adapter/ws-redis.adapter.ts
+++ b/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;
private redisConfig: RedisConfig;
+ private pubClient: Redis;
+ private subClient: Redis;
async connectToRedis(): Promise {
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 {
+ 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;
+ }
}
diff --git a/apps/server/test/integration/page-embeddable-ids-lockstep.int-spec.ts b/apps/server/test/integration/page-embeddable-ids-lockstep.int-spec.ts
new file mode 100644
index 00000000..9437e183
--- /dev/null
+++ b/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;
+ 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 {
+ 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 {
+ 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);
+ });
+});
diff --git a/packages/editor-ext/src/index.ts b/packages/editor-ext/src/index.ts
index 08888ddf..a2f1d0eb 100644
--- a/packages/editor-ext/src/index.ts
+++ b/packages/editor-ext/src/index.ts
@@ -25,6 +25,7 @@ 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";
diff --git a/packages/editor-ext/src/lib/image/image-markdown.test.ts b/packages/editor-ext/src/lib/image/image-markdown.test.ts
new file mode 100644
index 00000000..76803516
--- /dev/null
+++ b/packages/editor-ext/src/lib/image/image-markdown.test.ts
@@ -0,0 +1,68 @@
+import { describe, it, expect } from "vitest";
+import { generateJSON } from "@tiptap/html";
+import { Document } from "@tiptap/extension-document";
+import { Paragraph } from "@tiptap/extension-paragraph";
+import { Text } from "@tiptap/extension-text";
+import { htmlToMarkdown } from "../markdown/utils/turndown.utils";
+import { markdownToHtml } from "../markdown/utils/marked.utils";
+import { TiptapImage } from "./image";
+
+// Minimal schema for parsing markdownToHtml output back to JSON (mirrors
+// image.spec.ts), so we can assert the recovered caption EXACTLY.
+const parseExtensions = [Document, Paragraph, Text, TiptapImage];
+
+// Lossless markdown round-trip for image captions (issue #221). An image WITH a
+// caption can't be expressed as ``, so it is emitted as a raw
+// (carrying data-caption) wrapped in a block , the same trick the
+// rule uses. marked passes the raw HTML through, so markdownToHtml keeps the
+// data-caption, and the image extension's parseHTML restores the attribute.
+describe("image caption markdown round-trip", () => {
+ it("HTML -> Markdown emits a raw for captioned images", () => {
+ const html = `
`;
+ const md = htmlToMarkdown(html);
+ expect(md).toContain("data-caption=\"A grey cat\"");
+ expect(md).toContain('src="/files/a.png"');
+ expect(md).toContain('alt="cat"');
+ // It must NOT degrade to the lossy ![]() form.
+ expect(md).not.toContain("![cat]");
+ });
+
+ it("Markdown -> HTML restores data-caption on the ", async () => {
+ const html = `
`;
+ const md = htmlToMarkdown(html);
+ const back = await markdownToHtml(md);
+ expect(back).toContain('data-caption="A grey cat"');
+ expect(back).toContain('src="/files/a.png"');
+ });
+
+ it("special characters in the caption survive the round-trip (escaped)", async () => {
+ // The source caption is the decoded string `Tom & "Jerry"` (both an `&` and
+ // a `"`). escapeHtmlAttr must encode `&` -> `&` and `"` -> `"`.
+ const html = `
`;
+ const md = htmlToMarkdown(html);
+
+ // (a) The intermediate Markdown must carry the EXACT escaped attribute. This
+ // fails if escapeHtmlAttr stopped escaping `"` (attribute break-out:
+ // data-caption="Tom & "Jerry"") or double-encoded `&` (`&`).
+ expect(md).toContain('data-caption="Tom & "Jerry""');
+
+ const back = await markdownToHtml(md);
+ expect(back).toContain("data-caption=");
+ expect(back).toContain("Jerry");
+ expect(back).toContain("Tom");
+
+ // (b) Re-parse the rendered HTML through the image extension's parseHTML and
+ // assert the recovered caption is EXACTLY the original (no corruption, loss,
+ // or double-encoding).
+ const json = generateJSON(back, parseExtensions);
+ expect(json.content?.[0]?.attrs?.caption).toBe('Tom & "Jerry"');
+ });
+
+ it("caption-less images stay a clean  with no raw HTML", () => {
+ const html = `
`;
+ const md = htmlToMarkdown(html);
+ expect(md).toContain("");
+ expect(md).not.toContain("data-caption");
+ expect(md).not.toContain(" . If this mapping drifts, captions saved to HTML
+// (and thus to native storage / search / markdown) are silently lost.
+const extensions = [Document, Paragraph, Text, TiptapImage];
// applyAlignment is a pure DOM mutation: it sets the float / padding /
// justify-content / data-image-align on an image node-view container per the
@@ -65,3 +76,56 @@ describe("applyAlignment", () => {
expect(el.style.justifyContent).toBe("flex-start");
});
});
+
+describe("image schema", () => {
+ it("registers the image node and keeps it an atom", () => {
+ const schema = getSchema(extensions);
+ expect(schema.nodes.image).toBeTruthy();
+ expect(schema.nodes.image.spec.atom).toBe(true);
+ });
+});
+
+describe("image caption parse/render round-trip", () => {
+ it("recovers caption from data-caption on parse (HTML -> JSON)", () => {
+ const html = ` `;
+ const json = generateJSON(html, extensions);
+
+ const node = json.content?.[0];
+ expect(node?.type).toBe("image");
+ expect(node?.attrs?.caption).toBe("A grey cat");
+ expect(node?.attrs?.alt).toBe("cat");
+ });
+
+ it("emits data-caption on render when set (JSON -> HTML)", () => {
+ const json = {
+ type: "doc",
+ content: [
+ {
+ type: "image",
+ attrs: { src: "/files/a.png", alt: "cat", caption: "A grey cat" },
+ },
+ ],
+ };
+ const html = generateHTML(json, extensions);
+ expect(html).toContain('data-caption="A grey cat"');
+ });
+
+ it("omits data-caption when there is no caption (caption-less images stay clean)", () => {
+ const json = {
+ type: "doc",
+ content: [{ type: "image", attrs: { src: "/files/a.png", alt: "cat" } }],
+ };
+ const html = generateHTML(json, extensions);
+ expect(html).not.toContain("data-caption");
+ });
+
+ it("full HTML -> JSON -> HTML round-trip preserves the caption", () => {
+ const html = ` `;
+ const json = generateJSON(html, extensions);
+ expect(json.content?.[0]?.attrs?.caption).toBe('Caption with & "quotes"');
+
+ const out = generateHTML(json, extensions);
+ const back = generateJSON(out, extensions);
+ expect(back.content?.[0]?.attrs?.caption).toBe('Caption with & "quotes"');
+ });
+});
diff --git a/packages/editor-ext/src/lib/image/image.ts b/packages/editor-ext/src/lib/image/image.ts
index 7856ecb6..9fd597d7 100644
--- a/packages/editor-ext/src/lib/image/image.ts
+++ b/packages/editor-ext/src/lib/image/image.ts
@@ -32,6 +32,7 @@ export interface ImageOptions extends DefaultImageOptions {
export interface ImageAttributes {
src?: string;
alt?: string;
+ caption?: string;
align?: string;
attachmentId?: string;
size?: number;
@@ -125,6 +126,13 @@ export const TiptapImage = Image.extend({
alt: attributes.alt,
}),
},
+ caption: {
+ default: undefined,
+ parseHTML: (element) => element.getAttribute("data-caption") || undefined,
+ // Emit data-caption only when set, so caption-less images stay clean.
+ renderHTML: (attributes: ImageAttributes) =>
+ attributes.caption ? { "data-caption": attributes.caption } : {},
+ },
attachmentId: {
default: undefined,
parseHTML: (element) => element.getAttribute("data-attachment-id"),
@@ -304,6 +312,10 @@ export const TiptapImage = Image.extend({
el.alt = updatedNode.attrs.alt || "";
}
+ if (updatedNode.attrs.caption !== currentNode.attrs.caption) {
+ applyCaption(updatedNode.attrs.caption);
+ }
+
const w = updatedNode.attrs.width;
const h = updatedNode.attrs.height;
if (w != null) {
@@ -335,6 +347,28 @@ export const TiptapImage = Image.extend({
const dom = nodeView.dom as HTMLElement;
+ // Re-parent the resizable wrapper into a so the caption sits BELOW
+ // the image, OUTSIDE nodeView.wrapper. onCommit measures the img's
+ // offsetHeight for the persisted height/aspectRatio, and the left/right
+ // resize handles span the wrapper — both must cover the image only. The
+ // stays the single flex child of the container, so applyAlignment
+ // and the float modes keep working. This path also drives read-only/share.
+ const figure = document.createElement("figure");
+ figure.style.margin = "0";
+ figure.style.display = "inline-block"; // shrink-to-fit to image width
+ figure.appendChild(nodeView.wrapper);
+ dom.appendChild(figure);
+
+ const figcaption = document.createElement("figcaption");
+ figcaption.className = "image-caption";
+ const applyCaption = (text?: string) => {
+ const value = (text || "").trim();
+ figcaption.textContent = value;
+ figcaption.style.display = value ? "block" : "none";
+ };
+ applyCaption(node.attrs.caption);
+ figure.appendChild(figcaption);
+
// Apply initial alignment
applyAlignment(dom, node.attrs.align || "center");
diff --git a/packages/editor-ext/src/lib/markdown/utils/spoiler.marked.test.ts b/packages/editor-ext/src/lib/markdown/utils/spoiler.marked.test.ts
new file mode 100644
index 00000000..32b77de9
--- /dev/null
+++ b/packages/editor-ext/src/lib/markdown/utils/spoiler.marked.test.ts
@@ -0,0 +1,128 @@
+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 (`… `), 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(
+ 'before hidden after
',
+ 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('hidden ');
+
+ // 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"');
+ });
+});
diff --git a/packages/editor-ext/src/lib/markdown/utils/turndown.dataloss.test.ts b/packages/editor-ext/src/lib/markdown/utils/turndown.dataloss.test.ts
index 431c92f2..9bec4e78 100644
--- a/packages/editor-ext/src/lib/markdown/utils/turndown.dataloss.test.ts
+++ b/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) =>
- `before
${inner}after
`;
+describe("htmlToMarkdown — custom nodes are preserved losslessly (#206 mdrt-2)", () => {
+ const wrap = (inner: string) => `before
${inner}after
`;
- it("CURRENTLY drops a pageBreak entirely (data loss)", () => {
+ it("preserves a pageBreak block on Markdown export", () => {
const md = htmlToMarkdown(
wrap('
'),
);
- // 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('
'),
);
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(
+ 'hi @Bob there
',
+ );
+ // 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(
+ 's Done
',
+ );
+ // 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('
'),
);
- // 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(/]*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('
'),
);
- // Desired: the referenced id survives so the block can be rebuilt.
- expect(md).toContain("abc");
- },
- );
+ const html = await markdownToHtml(md);
+ expect(html).toMatch(/
]*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(
'
hi @Bob there
',
);
- // Desired: the mention keeps its stable identity (data-id), not just text.
- expect(md).toContain("u1");
- },
- );
+ const html = await markdownToHtml(md);
+ expect(html).toMatch(/
]*data-type="mention"[^>]*>/);
+ expect(html).toContain("u1");
+ expect(html).toContain("Bob");
+ });
+
+ it("round-trips a status chip (keeps data-color)", async () => {
+ const md = htmlToMarkdown(
+ 's Done
',
+ );
+ const html = await markdownToHtml(md);
+ expect(html).toMatch(/]*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(
+ `hi @A & <B> "C" there
`,
+ );
+
+ // (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 & <B> "C""');
+ expect(md).toContain('>@A & <B> "C" ');
+ // And explicitly NOT the raw, tag-corrupting forms: a literal `` (would
+ // mean `<`/`>` escaping was dropped in either the attr or the text)...
+ expect(md).not.toContain("");
+ // ...nor the malformed attribute that an unescaped `"` would produce.
+ expect(md).not.toContain('data-label="A & <B> "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 & "C"');
+ expect(span!.textContent).toBe('@A & "C"');
+ });
+ });
});
diff --git a/packages/editor-ext/src/lib/markdown/utils/turndown.utils.ts b/packages/editor-ext/src/lib/markdown/utils/turndown.utils.ts
index 172786a3..1a7a677d 100644
--- a/packages/editor-ext/src/lib/markdown/utils/turndown.utils.ts
+++ b/packages/editor-ext/src/lib/markdown/utils/turndown.utils.ts
@@ -12,6 +12,14 @@ function sanitizeMdLinkText(value: string): string {
.replace(/[\r\n]+/g, ' ');
}
+// Escape a value placed inside a double-quoted HTML attribute (img src/alt/
+// data-caption in the raw-HTML image fallback). Only & and " are special in
+// that context; escaping them is idempotent because parse5/marked decode them
+// back on re-import.
+function escapeHtmlAttr(value: string): string {
+ return value.replace(/&/g, '&').replace(/"/g, '"');
+}
+
// Tags turndown treats as void (self-closing). Footnote references render as an
// empty whose meaning lives entirely in its data-id;
// without marking it void, turndown's blank-node removal drops it before our
@@ -43,6 +51,54 @@ function fillEmptyFootnoteRefs(html: string): string {
);
}
+/**
+ * `pageBreak` and `transclusionReference` are childless atom 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(
+ /
]*\bdata-type="(?:pageBreak|transclusionReference)"[^>]*)>\s*<\/div>/gi,
+ (_m, attrs) => `
`,
+ );
+}
+
+/** HTML-escape an attribute value so a re-emitted raw-HTML tag is well-formed. */
+function escapeHtmlAttr(value: string): string {
+ return value
+ .replace(/&/g, '&')
+ .replace(/"/g, '"')
+ .replace(//g, '>');
+}
+
+/** HTML-escape text placed inside a re-emitted raw-HTML element. */
+function escapeHtmlText(value: string): string {
+ return value
+ .replace(/&/g, '&')
+ .replace(//g, '>');
+}
+
+/**
+ * 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,16 +121,88 @@ 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('
', ' ');
}
+/**
+ * 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
\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
\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 `
${text} `;
+ },
+ });
+}
+
+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 `
${text} `;
+ },
+ });
+}
+
/**
* Serialize the `htmlEmbed` node to Markdown.
*
@@ -101,6 +229,29 @@ 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 `
… ` 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 `
${content} `;
+ },
+ });
+}
+
function listParagraph(turndownService: _TurndownService) {
turndownService.addRule('paragraph', {
filter: ['p'],
@@ -258,6 +409,17 @@ function image(turndownService: _TurndownService) {
replacement: function (_content: string, node: HTMLInputElement) {
const src = node.getAttribute('src') || '';
if (!src) return '';
+ const caption = node.getAttribute('data-caption') || '';
+ if (caption) {
+ // ![]() can't carry a caption, so emit a raw
wrapped in a block
+ //
. marked passes it through and the image extension's parseHTML
+ // restores the caption from data-caption.
+ const parts = [`src="${escapeHtmlAttr(src)}"`];
+ const alt = node.getAttribute('alt') || '';
+ if (alt) parts.push(`alt="${escapeHtmlAttr(alt)}"`);
+ parts.push(`data-caption="${escapeHtmlAttr(caption)}"`);
+ return `
`;
+ }
const alt = sanitizeMdLinkText(node.getAttribute('alt') || '');
const title = node.getAttribute('title') || '';
const titlePart = title ? ' "' + title.replace(/"/g, '\\"') + '"' : '';
diff --git a/packages/editor-ext/src/lib/recreate-transform/recreateTransform.test.ts b/packages/editor-ext/src/lib/recreate-transform/recreateTransform.test.ts
new file mode 100644
index 00000000..a30dc3d2
--- /dev/null
+++ b/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);
+ });
+});
diff --git a/packages/editor-ext/src/lib/spoiler/spoiler.ts b/packages/editor-ext/src/lib/spoiler/spoiler.ts
new file mode 100644
index 00000000..22a06f95
--- /dev/null
+++ b/packages/editor-ext/src/lib/spoiler/spoiler.ts
@@ -0,0 +1,74 @@
+import { Mark, markInputRule, mergeAttributes } from "@tiptap/core";
+
+export interface SpoilerOptions {
+ HTMLAttributes: Record
;
+}
+
+declare module "@tiptap/core" {
+ interface Commands {
+ 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)([^|]+)(?({
+ 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.
+});
diff --git a/packages/editor-ext/src/lib/table/utils/get-selection-range-in-column.test.ts b/packages/editor-ext/src/lib/table/utils/get-selection-range-in-column.test.ts
new file mode 100644
index 00000000..70e0eed6
--- /dev/null
+++ b/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();
+ });
+});
diff --git a/packages/editor-ext/src/lib/table/utils/move-column.test.ts b/packages/editor-ext/src/lib/table/utils/move-column.test.ts
new file mode 100644
index 00000000..da524b8f
--- /dev/null
+++ b/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);
+ });
+});
diff --git a/packages/editor-ext/src/lib/table/utils/move-row.test.ts b/packages/editor-ext/src/lib/table/utils/move-row.test.ts
new file mode 100644
index 00000000..462d98fd
--- /dev/null
+++ b/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);
+ });
+});
diff --git a/packages/editor-ext/src/lib/table/utils/table-test-helpers.ts b/packages/editor-ext/src/lib/table/utils/table-test-helpers.ts
new file mode 100644
index 00000000..326cb650
--- /dev/null
+++ b/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): 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;
diff --git a/packages/editor-ext/src/lib/table/utils/table-utils.test.ts b/packages/editor-ext/src/lib/table/utils/table-utils.test.ts
index d8c964a2..70f59996 100644
--- a/packages/editor-ext/src/lib/table/utils/table-utils.test.ts
+++ b/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): 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)[][] =>
diff --git a/packages/editor-ext/src/lib/unique-id/unique-id.util.test.ts b/packages/editor-ext/src/lib/unique-id/unique-id.util.test.ts
index 24d30408..64603b5b 100644
--- a/packages/editor-ext/src/lib/unique-id/unique-id.util.test.ts
+++ b/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);
+ });
});
diff --git a/packages/editor-ext/tsconfig.json b/packages/editor-ext/tsconfig.json
index 5fcc2435..e9c235bd 100644
--- a/packages/editor-ext/tsconfig.json
+++ b/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"
]
}
diff --git a/packages/mcp/README.md b/packages/mcp/README.md
index be83177d..ecedb778 100644
--- a/packages/mcp/README.md
+++ b/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` /
diff --git a/packages/mcp/README.ru.md b/packages/mcp/README.ru.md
index e9f21569..546b6713 100644
--- a/packages/mcp/README.ru.md
+++ b/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` /
diff --git a/packages/mcp/src/index.ts b/packages/mcp/src/index.ts
index 39f48194..e0eb4f69 100644
--- a/packages/mcp/src/index.ts
+++ b/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",
diff --git a/packages/mcp/src/lib/auth-utils.ts b/packages/mcp/src/lib/auth-utils.ts
index d677be25..a3a3b652 100644
--- a/packages/mcp/src/lib/auth-utils.ts
+++ b/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.
diff --git a/packages/mcp/src/lib/docmost-schema.ts b/packages/mcp/src/lib/docmost-schema.ts
index 9a035f6f..07da5609 100644
--- a/packages/mcp/src/lib/docmost-schema.ts
+++ b/packages/mcp/src/lib/docmost-schema.ts
@@ -308,6 +308,26 @@ 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.
*
@@ -1178,7 +1198,26 @@ export const docmostExtensions = [
heading: {},
link: { openOnClick: false },
}),
- Image.configure({ inline: false }),
+ // Stock @tiptap/extension-image has no caption attribute, so a round-trip
+ // through this schema would drop the data-caption the client TiptapImage
+ // emits. Mirror editor-ext image.ts: add a caption attribute that parses
+ // data-caption and re-renders it only when set (caption-less images stay
+ // clean), keeping the MCP markdown round-trip lossless.
+ Image.extend({
+ addAttributes() {
+ const parent = this.parent?.() ?? {};
+ return {
+ ...parent,
+ caption: {
+ default: undefined,
+ parseHTML: (el: HTMLElement) =>
+ el.getAttribute("data-caption") || undefined,
+ renderHTML: (attrs: Record) =>
+ attrs.caption ? { "data-caption": attrs.caption } : {},
+ },
+ };
+ },
+ }).configure({ inline: false }),
TaskList,
TaskItem.configure({ nested: true }),
// Highlight stores its color unescaped and Docmost interpolates it into
@@ -1208,6 +1247,7 @@ export const docmostExtensions = [
// generateJSON drops , defeating the color import.
TextStyle,
Comment,
+ Spoiler,
Callout,
Table,
TableRow,
diff --git a/packages/mcp/src/lib/markdown-converter.ts b/packages/mcp/src/lib/markdown-converter.ts
index 4e35c995..f9758ace 100644
--- a/packages/mcp/src/lib/markdown-converter.ts
+++ b/packages/mcp/src/lib/markdown-converter.ts
@@ -167,6 +167,12 @@ 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 = `${textContent} `;
+ break;
}
}
}
@@ -228,16 +234,26 @@ export function convertProseMirrorToMarkdown(content: any): string {
// a bare "\n" would be reimported as a soft break and lost.
return " \n";
- case "image":
+ case "image": {
const imgAlt = node.attrs?.alt || "";
+ const imgCaption = node.attrs?.caption || "";
+ if (imgCaption) {
+ // ![]() can't carry a caption, so (symmetric to video) emit a raw
+ // wrapped in a block . On import marked.parse keeps the raw
+ // HTML and generateJSON runs the image extension's parseHTML, which
+ // restores the caption from data-caption.
+ const parts: string[] = [`src="${escapeAttr(node.attrs?.src ?? "")}"`];
+ if (imgAlt) parts.push(`alt="${escapeAttr(imgAlt)}"`);
+ parts.push(`data-caption="${escapeAttr(imgCaption)}"`);
+ return `
`;
+ }
// Neutralize characters that could break out of the markdown image
// URL: spaces/newlines and parentheses would terminate the (...) target
// and let a stored src inject following markdown/HTML. Percent-encode
// them so the URL stays a single inert token.
const imgSrc = encodeMdUrl(node.attrs?.src);
- // No "caption" attribute exists in the Docmost image schema, so we do
- // not emit one (the previous caption branch was dead).
return ``;
+ }
case "video": {
// Emit the schema-matching
element so generateJSON rebuilds the
@@ -678,6 +694,8 @@ export function convertProseMirrorToMarkdown(content: any): string {
const attrs = node.attrs || {};
const parts: string[] = [`src="${escapeAttr(attrs.src ?? "")}"`];
if (attrs.alt) parts.push(`alt="${escapeAttr(attrs.alt)}"`);
+ if (attrs.caption)
+ parts.push(`data-caption="${escapeAttr(attrs.caption)}"`);
if (attrs.title) parts.push(`title="${escapeAttr(attrs.title)}"`);
if (attrs.width != null) parts.push(`width="${escapeAttr(attrs.width)}"`);
if (attrs.height != null) parts.push(`height="${escapeAttr(attrs.height)}"`);
diff --git a/packages/mcp/test-e2e.mjs b/packages/mcp/test-e2e.mjs
index 97e66b88..7d3d78bb 100644
--- a/packages/mcp/test-e2e.mjs
+++ b/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);
diff --git a/packages/mcp/test/unit/auth-cookie.test.mjs b/packages/mcp/test/unit/auth-cookie.test.mjs
new file mode 100644
index 00000000..92206cf4
--- /dev/null
+++ b/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/,
+ );
+});
diff --git a/packages/mcp/test/unit/client-host-contract.test.mjs b/packages/mcp/test/unit/client-host-contract.test.mjs
new file mode 100644
index 00000000..424bfbb7
--- /dev/null
+++ b/packages/mcp/test/unit/client-host-contract.test.mjs
@@ -0,0 +1,212 @@
+import { test } from "node:test";
+import assert from "node:assert/strict";
+import { readFileSync } from "node:fs";
+import { fileURLToPath } from "node:url";
+import { dirname, resolve } from "node:path";
+
+import { DocmostClient } from "../../build/index.js";
+
+// Drift guard for the THIRD hand-written layer of the AI tool set (issue #193,
+// layer 3): the in-app server hand-mirrors the DocmostClient method signatures
+// it consumes as the `DocmostClientLike` interface in
+// apps/server/src/core/ai-chat/tools/docmost-client.loader.ts ("Signatures here
+// mirror that file exactly"). That mirror lives across the ESM(mcp)/CJS(server)
+// boundary and the package ships NO .d.ts, so the server typecheck cannot verify
+// the names against the real class — a rename/removal in client.ts would surface
+// only as a runtime "x is not a function" inside an agent tool call.
+//
+// SCOPE: this guard checks the method-NAME set only, not signatures. It pins the
+// contract from the mcp side (ESM, where the real class is directly importable):
+// every method the embedding host depends on MUST exist as a function on a real
+// DocmostClient instance. If you rename/remove a client method, this fails here
+// AND you must update DocmostClientLike to match. It does NOT verify parameter or
+// return-type parity — signature drift between the hand-mirror and client.ts can
+// still ship silently; full signature/type parity is the deferred staged-plan
+// item below.
+//
+// Keep the HOST_CONTRACT_METHODS NAME list aligned with the method NAMES declared
+// in the server's DocmostClientLike interface (the in-app per-user tool adapter
+// only — it is a SUBSET of the DocmostClient surface — covers only what the in-app adapter
+// consumes; the standalone MCP transport (packages/mcp/src/index.ts) calls additional
+// client methods (insertImage/replaceImage/deleteComment/updateComment/insertFootnote)
+// that this guard does NOT track — the MCP transport's own typecheck covers those). Full type-derivation
+// of DocmostClientLike from this class is deferred (see the staged plan in
+// docmost-client.loader.ts): the package emits no declarations and the real
+// (inferred, concrete) return types conflict with the host's loose
+// `Record` + `as`-cast result handling.
+const HOST_CONTRACT_METHODS = [
+ // read
+ "search",
+ "getPage",
+ "getWorkspace",
+ "getSpaces",
+ "listPages",
+ "listSidebarPages",
+ "getOutline",
+ "getPageJson",
+ "getNode",
+ "getTable",
+ "listComments",
+ "getComment",
+ "checkNewComments",
+ "listShares",
+ "listPageHistory",
+ "getPageHistory",
+ "diffPageVersions",
+ "exportPageMarkdown",
+ // write (page)
+ "createPage",
+ "updatePage",
+ "renamePage",
+ "movePage",
+ "deletePage",
+ "editPageText",
+ "patchNode",
+ "insertNode",
+ "deleteNode",
+ "updatePageJson",
+ "tableInsertRow",
+ "tableDeleteRow",
+ "tableUpdateCell",
+ "copyPageContent",
+ "importPageMarkdown",
+ "sharePage",
+ "unsharePage",
+ "restorePageVersion",
+ "transformPage",
+ // write (comment)
+ "createComment",
+ "resolveComment",
+];
+
+test("DocmostClient implements every method the in-app DocmostClientLike mirror declares", () => {
+ // The constructor is side-effect-free (no network/login on construction): it
+ // only stores config and creates an axios instance, so it is safe to build a
+ // throwaway instance here with a dummy token provider.
+ const client = new DocmostClient({
+ apiUrl: "http://127.0.0.1:1/api",
+ getToken: async () => "test-token",
+ });
+
+ const missing = HOST_CONTRACT_METHODS.filter(
+ (name) => typeof client[name] !== "function",
+ );
+
+ assert.deepEqual(
+ missing,
+ [],
+ `DocmostClient is missing host-contract method(s): ${missing.join(", ")}. ` +
+ `Update packages/mcp/src/client.ts and/or the server's DocmostClientLike ` +
+ `interface (apps/server/src/core/ai-chat/tools/docmost-client.loader.ts) ` +
+ `so the hand-mirrored method NAMES stay aligned (this guards names only, ` +
+ `not signatures).`,
+ );
+});
+
+test("HOST_CONTRACT_METHODS has no duplicates", () => {
+ assert.equal(
+ new Set(HOST_CONTRACT_METHODS).size,
+ HOST_CONTRACT_METHODS.length,
+ );
+});
+
+// Parse the method names declared in the server's `DocmostClientLike` interface
+// body. We read the .ts source as plain text (no TS compiler dep, and the file
+// lives in the CJS server tree across the ESM boundary): scan from the
+// `export interface DocmostClientLike {` line to its closing brace at column 0,
+// matching member-signature lines like ` methodName(`. Nested param-object
+// braces (`opts: { ... }`) are indented, so only the interface's own closing
+// `}` (column 0) ends the scan.
+function parseDocmostClientLikeMethods() {
+ const here = dirname(fileURLToPath(import.meta.url));
+ // packages/mcp/test/unit -> repo root is four levels up.
+ const loaderPath = resolve(
+ here,
+ "../../../../apps/server/src/core/ai-chat/tools/docmost-client.loader.ts",
+ );
+ let source;
+ try {
+ source = readFileSync(loaderPath, "utf8");
+ } catch (err) {
+ if (err && err.code === "ENOENT") {
+ throw new Error(
+ `Expected monorepo layout; server tree at ${loaderPath} not found. ` +
+ `This drift-guard reads the server's DocmostClientLike interface via a ` +
+ `fixed relative path and must run from inside the monorepo checkout.`,
+ );
+ }
+ throw err;
+ }
+ const lines = source.split(/\r?\n/);
+
+ const startIdx = lines.findIndex((l) =>
+ /^export interface DocmostClientLike\s*\{/.test(l),
+ );
+ assert.notEqual(
+ startIdx,
+ -1,
+ `Could not find "export interface DocmostClientLike {" in ${loaderPath}. ` +
+ `If the interface was renamed/moved, update this drift-guard test.`,
+ );
+
+ const methods = [];
+ let closed = false;
+ // Track whether we are inside a `/* ... */` block comment. Inner lines of a
+ // block comment need NOT start with `*`, so a `name(` line inside one would be
+ // falsely parsed as an interface method without this. (`//` line comments can
+ // never match the method regex below since they start with `/`.)
+ let inBlockComment = false;
+ for (let i = startIdx + 1; i < lines.length; i++) {
+ const line = lines[i];
+ if (inBlockComment) {
+ // Stay in the block until we see its closing `*/`.
+ if (line.includes("*/")) inBlockComment = false;
+ continue;
+ }
+ // Enter a block comment only when it opens without closing on the same line;
+ // a self-contained `/* ... */` on one line cannot precede a method name we
+ // care about (such lines start with `/`, so the method regex won't match).
+ if (line.includes("/*") && !line.includes("*/")) {
+ inBlockComment = true;
+ continue;
+ }
+ if (/^\}/.test(line)) {
+ closed = true;
+ break;
+ }
+ // Method-name match: a TS identifier (letters/digits/`_`/`$`, not starting
+ // with a digit) optionally followed by a generic clause (`method(`), then
+ // the opening paren of the signature.
+ const m = /^\s*([A-Za-z_$][A-Za-z0-9_$]*)\s*(?:<[^>]*>)?\(/.exec(line);
+ if (m) methods.push(m[1]);
+ }
+ assert.ok(
+ closed,
+ `Did not find the closing brace of DocmostClientLike in ${loaderPath}.`,
+ );
+ assert.ok(
+ methods.length > 0,
+ `Parsed zero methods from DocmostClientLike in ${loaderPath} — the parser ` +
+ `is likely out of date with the interface formatting.`,
+ );
+ return methods;
+}
+
+// The point of the guard is to protect the DocmostClientLike mirror <-> client.ts
+// link, but HOST_CONTRACT_METHODS is itself a HAND-COPY of that interface kept in
+// sync manually. The list<->interface link must be tested too: a method consumed
+// by the adapter and added to DocmostClientLike but forgotten here (or removed
+// from the interface but left here) would otherwise escape both the server
+// typecheck (pkg emits no .d.ts) and the first test above (name not in the list).
+// Assert the two agree BOTH ways.
+test("HOST_CONTRACT_METHODS exactly mirrors the server's DocmostClientLike interface", () => {
+ const interfaceMethods = parseDocmostClientLikeMethods();
+ assert.deepEqual(
+ [...HOST_CONTRACT_METHODS].sort(),
+ [...interfaceMethods].sort(),
+ `HOST_CONTRACT_METHODS has drifted from the DocmostClientLike interface in ` +
+ `apps/server/src/core/ai-chat/tools/docmost-client.loader.ts. Add/remove ` +
+ `method names in HOST_CONTRACT_METHODS so it lists EXACTLY the methods ` +
+ `declared in that interface (both directions are checked).`,
+ );
+});
diff --git a/packages/mcp/test/unit/comment-anchor-apply.test.mjs b/packages/mcp/test/unit/comment-anchor-apply.test.mjs
new file mode 100644
index 00000000..46049c02
--- /dev/null
+++ b/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);
+});
diff --git a/packages/mcp/test/unit/docmost-md-roundtrip.test.mjs b/packages/mcp/test/unit/docmost-md-roundtrip.test.mjs
index c80fbd53..798bac10 100644
--- a/packages/mcp/test/unit/docmost-md-roundtrip.test.mjs
+++ b/packages/mcp/test/unit/docmost-md-roundtrip.test.mjs
@@ -167,6 +167,38 @@ 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, /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",
diff --git a/packages/mcp/test/unit/markdown-converter.test.mjs b/packages/mcp/test/unit/markdown-converter.test.mjs
index 852de894..051a7e88 100644
--- a/packages/mcp/test/unit/markdown-converter.test.mjs
+++ b/packages/mcp/test/unit/markdown-converter.test.mjs
@@ -149,3 +149,37 @@ test("empty task item still emits its marker", () => {
assert.equal(convertProseMirrorToMarkdown(input), "- [ ]\n- [x]");
});
+
+// Image captions (issue #221). An image WITHOUT a caption stays the lossy-free
+// ``; WITH a caption it is emitted as a raw
+// wrapped in a block (symmetric to video) so the round-trip md -> html ->
+// json restores the caption via the image extension's parseHTML.
+test("image without a caption emits plain ", () => {
+ const input = doc({
+ type: "image",
+ attrs: { src: "/files/a.png", alt: "cat" },
+ });
+ assert.equal(convertProseMirrorToMarkdown(input), "");
+});
+
+test("image with a caption emits a raw
in a block div", () => {
+ const input = doc({
+ type: "image",
+ attrs: { src: "/files/a.png", alt: "cat", caption: "A grey cat" },
+ });
+ assert.equal(
+ convertProseMirrorToMarkdown(input),
+ '
',
+ );
+});
+
+test("image caption escapes & and \" in the data-caption attribute", () => {
+ const input = doc({
+ type: "image",
+ attrs: { src: "/files/a.png", caption: 'Tom & "Jerry"' },
+ });
+ assert.equal(
+ convertProseMirrorToMarkdown(input),
+ '
',
+ );
+});
diff --git a/packages/mcp/test/unit/media-roundtrip-attrs.test.mjs b/packages/mcp/test/unit/media-roundtrip-attrs.test.mjs
new file mode 100644
index 00000000..37668fe0
--- /dev/null
+++ b/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");
+});
diff --git a/packages/mcp/test/unit/media-roundtrip.test.mjs b/packages/mcp/test/unit/media-roundtrip.test.mjs
index 01c6d25f..9ef99602 100644
--- a/packages/mcp/test/unit/media-roundtrip.test.mjs
+++ b/packages/mcp/test/unit/media-roundtrip.test.mjs
@@ -142,3 +142,31 @@ test("round-trip: pdf node survives markdown export with src + name + attachment
assert.equal(found[0].attrs?.name, "x.pdf");
assert.equal(found[0].attrs?.attachmentId, "a4");
});
+
+// The converter emits captioned images as a raw
; for
+// the caption to survive the PM -> markdown -> PM round-trip the docmost-schema
+// Image node must parse data-caption back into the `caption` attr. Without that
+// (stock @tiptap/extension-image), the caption is silently lost — these guard
+// the "lossless" claim.
+test("round-trip: image caption survives markdown export (data-caption restored)", async () => {
+ const found = await roundtrip(
+ { type: "image", attrs: { src: "/api/files/cat.png", alt: "cat", caption: "A grey cat" } },
+ "image",
+ );
+ assert.equal(found.length, 1, "image node should survive");
+ assert.equal(found[0].attrs?.src, "/api/files/cat.png");
+ assert.equal(found[0].attrs?.caption, "A grey cat", "caption must round-trip");
+});
+
+test("round-trip: image caption with special chars survives markdown export", async () => {
+ const found = await roundtrip(
+ { type: "image", attrs: { src: "/api/files/cat.png", caption: 'Tom & "Jerry"' } },
+ "image",
+ );
+ assert.equal(found.length, 1, "image node should survive");
+ assert.equal(
+ found[0].attrs?.caption,
+ 'Tom & "Jerry"',
+ "special-char caption must round-trip unescaped",
+ );
+});
diff --git a/packages/mcp/test/unit/recreate-transform-drift.test.mjs b/packages/mcp/test/unit/recreate-transform-drift.test.mjs
new file mode 100644
index 00000000..b950cdaf
--- /dev/null
+++ b/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}"`,
+ );
+ });
+}
diff --git a/packages/mcp/test/unit/roundtrip.test.mjs b/packages/mcp/test/unit/roundtrip.test.mjs
index 56852cee..1b80e554 100644
--- a/packages/mcp/test/unit/roundtrip.test.mjs
+++ b/packages/mcp/test/unit/roundtrip.test.mjs
@@ -82,6 +82,24 @@ test("round-trip: image inside a column survives as an image node (not literal m
assert.ok(!JSON.stringify(out).includes("![pic]"), "image must not become literal markdown text");
});
+test("round-trip: captioned image inside a column preserves its caption (imageToHtml branch)", async () => {
+ // A captioned image in a column is emitted via the imageToHtml helper (raw
+ // HTML container), a different path from the top-level image case. Special
+ // chars in the caption exercise attribute escaping on the way out and in.
+ const caption = 'Tom & "Jerry"';
+ const input = doc({
+ type: "columns",
+ content: [
+ { type: "column", content: [{ type: "image", attrs: { src: "/api/files/a/p.png", alt: "pic", caption } }] },
+ { type: "column", content: [para(text("right"))] },
+ ],
+ });
+ const out = await roundtrip(input);
+ const imgs = findNodes(out, "image");
+ assert.equal(imgs.length, 1, "captioned image inside a column must survive");
+ assert.equal(imgs[0].attrs?.caption, caption, "caption (incl. special chars) must be preserved");
+});
+
test("round-trip: blockquote inside a column survives as a blockquote node", async () => {
const input = doc({
type: "columns",