Compare commits

...

5 Commits

Author SHA1 Message Date
agent_coder 0f5f048ca2 fix(mcp): pre-validate node JSON против схемы + путь битого узла (#409, остаток Фазы 1)
Структурные редакторы (patchNode/insertNode/updatePageJson/transformPage) кидали
опаковый Yjs-крах на агентском JSON с вложенным узлом без/с неизвестным `type`:
«Failed to encode document to Yjs (fromJSON): Unknown node type: undefined» —
ГЛУБОКО в энкодере, уже ПОСЛЕ открытия collab-сессии, а хинт мислейблил это как
проблему атрибута. Агент ретраил вслепую (~34 краха в истории 06-17…07-07).

- findInvalidNode(doc) в prosemirror-markdown/node-ops.ts: DFS по content,
  возвращает {path, summary} первого узла с отсутствующим/не-строковым `type`
  или типом/маркой вне схемы. Множество имён — из getSchema(docmostExtensions),
  ТОГО ЖЕ, из которого энкод-путь строит docmostSchema → «известный тип»
  обходчика ровно то, что примет PMNode.fromJSON/toYdoc (сверено на 45 узлах +
  12 марках, ни ложных положительных, ни пропуска краш-типа).
- unstorableYjsError: findInvalidNode ПЕРВЫМ (node-shape крах больше не
  мислейблится как атрибут), затем findUnstorableAttr, generic-фраза последней.
- assertValidNodeShape(op, node) ДО getCollabTokenWithReauth/mutatePageContent
  в patchNode/insertNode/updatePageJson: fail-fast — collab-сессия не
  открывается, page-lock не берётся, сообщение детерминировано (mock-тест
  ассертит collabTokenFetched===false на битом пути). tableUpdateCell не тронут
  (строит абзац из plain text через makeCellParagraph, агентский JSON не глотает).
- Описания patch_node/insert_node/update_page_json: каждый узел, включая
  вложенные, несёт строковый `type` из схемы; текст-листы {"type":"text",...}.

sanitizeForYjs (стрип undefined-атрибутов) сохранён — другой класс отказа.
Внутреннее ревью: APPROVE WITH SUGGESTIONS — schema-fidelity/fail-fast/
no-false-positive/precedence подтверждены; замечания необязательны (тест
перечисления схемы, depth-guard безобиден т.к. энкодер падает раньше).
prosemirror-markdown vitest 726/726, mcp node --test 613/613.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-10 10:49:51 +03:00
vvzvlad f8d37d8956 Merge pull request 'fix(mcp): курсорная пагинация — устранить тихую потерю страниц в list_pages/check_new_comments (#442)' (#451) from fix/442-cursor-pagination into develop
Reviewed-on: #451
2026-07-10 07:27:31 +03:00
vvzvlad 0108dec0e6 Merge pull request 'feat(mcp): детерминированная нормализация текста сносок + склейка форков' (#422) from feat/419-footnote-normalize into develop
Reviewed-on: #422
2026-07-10 07:08:50 +03:00
agent_coder f750a509c2 fix(mcp): не нормализовать текст под маркой code в сносках (порча code-литералов)
Ревью #422: безусловная нормализация переписывала в ASCII и текст inline-code
внутри сносок (кавычки/тире/спецпробелы) — для code-литерала это не типографика,
а изменение смысла (эмпирически на raw-JSON путях: code-нода "a—b «x»" -> "a-b").
normalizeDefinitionText теперь пропускает текст-ноды с маркой code (verbatim), и
краевой trim определения тоже не трогает крайние code-ноды. footnoteMergeKey
читает сырой текст code-нод -> сноски, различающиеся только глифами в коде, НЕ
сливаются, а форки в прозе по-прежнему сливаются. Тесты: code verbatim +
непослияние по code-глифам.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-10 07:03:34 +03:00
agent_coder d4581a096f feat(mcp): детерминированная нормализация текста сносок + склейка форков по нормализованному тексту
Новый чистый модуль footnote-normalize-merge.ts (normalizeAndMergeFootnotes):
нормализует текст определений сносок (типографские кавычки->ASCII, тире->'-',
NBSP/спецпробелы->пробел, схлопывание пробелов, trim) и сливает определения с
совпавшим нормализованным текстом, перевешивая ссылки на канонический id;
дубли-сироты добивает canonicalizeFootnotes. Ключ слияния attrs-aware
(footnoteMergeKey/stableAttrs) — сноски с одинаковым текстом, но разными attrs
марок (напр. link.href) НЕ сливаются (защита от потери target). Пасс вызывается
строго ПЕРЕД canonicalizeFootnotes на 5 write-путях MCP (markdown-импорт,
updatePageJson, copyPageContent, docmost_transform, insertInlineFootnote).
Глиф-карты продублированы из comment-anchor.ts (там private+завязаны на golden).
Идемпотентен, чистый (deep-clone), scope строго внутри footnoteDefinition.

closes #419

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-10 07:03:34 +03:00
11 changed files with 1227 additions and 8 deletions
+48 -2
View File
@@ -42,6 +42,7 @@ import {
insertTableRow, insertTableRow,
deleteTableRow, deleteTableRow,
updateTableCell, updateTableCell,
findInvalidNode,
} from "@docmost/prosemirror-markdown"; } from "@docmost/prosemirror-markdown";
import { searchInDoc, SearchOptions } from "./lib/page-search.js"; import { searchInDoc, SearchOptions } from "./lib/page-search.js";
import { withPageLock } from "./lib/page-lock.js"; import { withPageLock } from "./lib/page-lock.js";
@@ -82,6 +83,7 @@ import {
canonicalizeFootnotes, canonicalizeFootnotes,
insertInlineFootnote, insertInlineFootnote,
} from "./lib/transforms.js"; } from "./lib/transforms.js";
import { normalizeAndMergeFootnotes } from "./lib/footnote-normalize-merge.js";
import vm from "node:vm"; import vm from "node:vm";
// Supported image types, kept as two lookup tables so both a local file // Supported image types, kept as two lookup tables so both a local file
@@ -1659,6 +1661,27 @@ export class DocmostClient {
} }
} }
/**
* Pre-write SHAPE gate (#409). Walk the WHOLE node tree with the shared
* `findInvalidNode` and throw a rich, path-anchored error the instant a nested
* node has an absent/unknown `type` (or an unknown mark) — the exact shape that
* otherwise surfaces DEEP in the Yjs encode as the cryptic
* `Unknown node type: undefined`, but only AFTER a collab session was opened
* and a page lock taken. Calling this BEFORE `getCollabTokenWithReauth` /
* `mutatePageContent` fails fast: no collab connection, no lock, deterministic
* message. `op` names the tool for the message prefix (e.g. "patch_node").
*
* `findInvalidNode` derives its "known type" set from the very same
* `docmostExtensions` the encode path uses, so a node this gate accepts is one
* the encoder will accept too.
*/
private assertValidNodeShape(op: string, node: any): void {
const bad = findInvalidNode(node);
if (bad) {
throw new Error(`${op}: invalid node — ${bad.summary}`);
}
}
/** /**
* Replace page content with a raw ProseMirror JSON document (lossless) and/or * Replace page content with a raw ProseMirror JSON document (lossless) and/or
* update its title. Both `doc` and `title` are optional, but at least one must * update its title. Both `doc` and `title` are optional, but at least one must
@@ -1710,6 +1733,12 @@ export class DocmostClient {
// page on overwrite. // page on overwrite.
this.validateDocStructure(doc); this.validateDocStructure(doc);
// #409: beyond the string-`type` check above, reject a nested node whose
// `type` is a string but NOT a known Docmost schema node (a typo/unknown
// block) — the same `Unknown node type` the encoder throws — with a rich,
// path-anchored message, still BEFORE any collab connection.
this.assertValidNodeShape("update_page_json", doc);
// Sanitize URLs before writing. This closes the JSON-path bypass: unlike // Sanitize URLs before writing. This closes the JSON-path bypass: unlike
// the markdown link path (which TipTap sanitizes), raw JSON could otherwise // the markdown link path (which TipTap sanitizes), raw JSON could otherwise
// inject javascript:/data: link hrefs or media srcs straight into the doc. // inject javascript:/data: link hrefs or media srcs straight into the doc.
@@ -1719,6 +1748,8 @@ export class DocmostClient {
// leave footnotes out of order, orphaned, or in multiple lists — the bottom // leave footnotes out of order, orphaned, or in multiple lists — the bottom
// list + numbering are always derived from reference order. No-op when the // list + numbering are always derived from reference order. No-op when the
// footnotes are already canonical. // footnotes are already canonical.
// #419: normalize + merge glyph-forked definitions before canonicalizing.
doc = normalizeAndMergeFootnotes(doc);
doc = canonicalizeFootnotes(doc); doc = canonicalizeFootnotes(doc);
// Write the BODY first, then the title (#159 split-brain): a failed body // Write the BODY first, then the title (#159 split-brain): a failed body
@@ -1983,7 +2014,8 @@ export class DocmostClient {
// footnotes before copying — a no-op on already-canonical source content, but // footnotes before copying — a no-op on already-canonical source content, but
// it guarantees a copy can never propagate a non-canonical footnote topology // it guarantees a copy can never propagate a non-canonical footnote topology
// to the target (parity with the other full-doc write paths). // to the target (parity with the other full-doc write paths).
const canonical = canonicalizeFootnotes(content); // #419: normalize + merge glyph-forked definitions before canonicalizing.
const canonical = canonicalizeFootnotes(normalizeAndMergeFootnotes(content));
const collabToken = await this.getCollabTokenWithReauth(); const collabToken = await this.getCollabTokenWithReauth();
// Open the TARGET collab doc by its canonical UUID, never the slugId (#260). // Open the TARGET collab doc by its canonical UUID, never the slugId (#260).
@@ -2127,6 +2159,14 @@ export class DocmostClient {
target.attrs.id = nodeId; target.attrs.id = nodeId;
} }
// #409: fail fast on a malformed node SHAPE (a nested child with an
// absent/unknown `type`, e.g. a text leaf written as `{"text":"foo"}` with
// no `"type":"text"`) BEFORE opening a collab session or taking the page
// lock — the root-only `typeof node.type === "string"` check above never
// sees nested children, and the encoder's `Unknown node type: undefined`
// would otherwise only surface after the connection.
this.assertValidNodeShape("patch_node", target);
const collabToken = await this.getCollabTokenWithReauth(); const collabToken = await this.getCollabTokenWithReauth();
// Open the collab doc by the canonical UUID, never the slugId (#260). // Open the collab doc by the canonical UUID, never the slugId (#260).
const pageUuid = await this.resolvePageId(pageId); const pageUuid = await this.resolvePageId(pageId);
@@ -2217,6 +2257,11 @@ export class DocmostClient {
} }
} }
// #409: fail fast on a malformed node SHAPE (a nested child with an
// absent/unknown `type`) BEFORE opening a collab session or taking the page
// lock — the root-only check above never sees nested children.
this.assertValidNodeShape("insert_node", node);
const collabToken = await this.getCollabTokenWithReauth(); const collabToken = await this.getCollabTokenWithReauth();
// Open the collab doc by the canonical UUID, never the slugId (#260). // Open the collab doc by the canonical UUID, never the slugId (#260).
const pageUuid = await this.resolvePageId(pageId); const pageUuid = await this.resolvePageId(pageId);
@@ -4249,7 +4294,8 @@ export class DocmostClient {
// path can leave footnotes out of order / orphaned / in a raw `[^id]` // path can leave footnotes out of order / orphaned / in a raw `[^id]`
// block. In a dryRun preview this may surface footnote edits the script // block. In a dryRun preview this may surface footnote edits the script
// author did not write (the canonicalizer tidied them) — that is expected. // author did not write (the canonicalizer tidied them) — that is expected.
const result = canonicalizeFootnotes(raw); // #419: normalize + merge glyph-forked definitions before canonicalizing.
const result = canonicalizeFootnotes(normalizeAndMergeFootnotes(raw));
newDoc = result; newDoc = result;
return result; return result;
}; };
+27 -3
View File
@@ -13,8 +13,13 @@ import { JSDOM } from "jsdom";
import { markdownToProseMirror } from "@docmost/prosemirror-markdown"; import { markdownToProseMirror } from "@docmost/prosemirror-markdown";
import { docmostExtensions, docmostSchema } from "./docmost-schema.js"; import { docmostExtensions, docmostSchema } from "./docmost-schema.js";
import { withPageLock } from "./page-lock.js"; import { withPageLock } from "./page-lock.js";
import { sanitizeForYjs, findUnstorableAttr } from "@docmost/prosemirror-markdown"; import {
sanitizeForYjs,
findUnstorableAttr,
findInvalidNode,
} from "@docmost/prosemirror-markdown";
import { canonicalizeFootnotes } from "./footnote-canonicalize.js"; import { canonicalizeFootnotes } from "./footnote-canonicalize.js";
import { normalizeAndMergeFootnotes } from "./footnote-normalize-merge.js";
import { VerifyReport } from "./diff.js"; import { VerifyReport } from "./diff.js";
import { acquireCollabSession } from "./collab-session.js"; import { acquireCollabSession } from "./collab-session.js";
@@ -27,11 +32,25 @@ export { markdownToProseMirror };
* place. `label` names the stage that failed (diagnostic). `sanitizeForYjs` * place. `label` names the stage that failed (diagnostic). `sanitizeForYjs`
* already stripped `undefined` attrs, so a remaining failure is pinpointed via * already stripped `undefined` attrs, so a remaining failure is pinpointed via
* `findUnstorableAttr`. * `findUnstorableAttr`.
*
* Diagnostics precedence (#409): the dominant crash here is
* `Unknown node type: undefined` — a nested node with an absent/unknown `type`
* (a SHAPE problem, e.g. `{"text":"foo"}` missing `"type":"text"`). That points
* at the node, not an attribute, so `findInvalidNode` is consulted FIRST and,
* on a hit, yields a path-anchored node-shape message. Only when the document
* shape is sound do we fall back to `findUnstorableAttr` (undefined/function/
* symbol/bigint attr values); the generic "attribute likely holds a value Yjs
* cannot store" sentence is the last resort.
*/ */
function unstorableYjsError(safe: any, label: string, e: unknown): Error { function unstorableYjsError(safe: any, label: string, e: unknown): Error {
const base = `Failed to encode document to Yjs (${label}): ${e instanceof Error ? e.message : String(e)}.`;
const badNode = findInvalidNode(safe);
if (badNode) {
return new Error(`${base} Invalid node: ${badNode.summary}`);
}
const bad = findUnstorableAttr(safe); const bad = findUnstorableAttr(safe);
return new Error( return new Error(
`Failed to encode document to Yjs (${label}): ${e instanceof Error ? e.message : String(e)}.${bad ? ` Offending attribute: ${bad}.` : " A node/mark attribute likely holds a value Yjs cannot store (e.g. undefined)."}`, `${base}${bad ? ` Offending attribute: ${bad}.` : " A node/mark attribute likely holds a value Yjs cannot store (e.g. undefined)."}`,
); );
} }
@@ -82,7 +101,12 @@ global.WebSocket = WebSocket;
export async function markdownToProseMirrorCanonical( export async function markdownToProseMirrorCanonical(
markdownContent: string, markdownContent: string,
): Promise<any> { ): Promise<any> {
return canonicalizeFootnotes(await markdownToProseMirror(markdownContent)); // #419: normalize + merge glyph-forked footnote definitions BEFORE
// canonicalizing, so the canonicalizer re-hangs references and drops the
// now-orphaned duplicate definitions.
return canonicalizeFootnotes(
normalizeAndMergeFootnotes(await markdownToProseMirror(markdownContent)),
);
} }
/** /**
@@ -0,0 +1,280 @@
/**
* Deterministic server-side NORMALIZATION + MERGE of footnote DEFINITIONS
* (MCP, PURE).
*
* Problem (#419): footnotes with the same meaning but different GLYPHS —
* typographic quotes («…»/“…”) vs ASCII "…", em/en-dash vs `-`, non-breaking
* space vs normal space, differing space counts — are not recognized as equal
* and "fork": two definitions appear where the author meant one. The existing
* de-dup paths miss this: `footnoteContentKey` (footnote-authoring.ts) only
* collapses ASCII whitespace (quotes/dashes/NBSP untouched), and
* `canonicalizeFootnotes` keys purely by `attrs.id` (the two forks have
* different ids), so neither glues the forks together.
*
* This pass fixes that DETERMINISTICALLY on the MCP write-paths (an LLM
* instruction gives no glue guarantee). It:
* 1. Normalizes the TEXT of every `footnoteDefinition`'s text nodes IN PLACE
* (typographic quotes -> ASCII "/', dashes -> `-`, NBSP & friends ->
* normal space, whitespace runs collapsed, whole-definition edges
* trimmed) — unconditionally, for ALL definitions, KEEPING their marks.
* 2. Computes a MERGE KEY per definition (normalized text + an ATTRS-AWARE
* inline-mark signature, via the local `footnoteMergeKey`), so notes that
* read the same but differ in formatting (bold vs plain) OR in a mark
* attribute (a `link` with a different `href`, differing `code`/`highlight`
* attrs) are NOT merged. See `footnoteMergeKey` for why this diverges from
* the shared type-only `footnoteContentKey`.
* 3. Maps every duplicate definition id to the FIRST (document-order)
* definition's id and re-hangs `footnoteReference` nodes onto it.
*
* Duplicate definitions keep their original ids but now have NO references, so
* the canonicalizer that runs immediately after this pass removes them as
* orphans and derives the single tail list + numbering. This pass therefore
* MUST run BEFORE `canonicalizeFootnotes(doc)` at every write-path call-site
* (see the enforcement rule in `footnote-canonicalize.ts`).
*
* Accepted tradeoff: the exact typographic glyphs of the SURVIVING footnote are
* rewritten to ASCII, in exchange for a GUARANTEED merge. Scope is strictly
* INSIDE `footnoteDefinition` — body text (normal paragraphs) is never touched.
*
* Pure: deep-clones its input, deterministic, idempotent (a re-run is a no-op —
* text is already normalized and references already point at the canonical id,
* so no spurious mutations / git-sync churn).
*/
const FOOTNOTE_DEFINITION_NAME = "footnoteDefinition";
const FOOTNOTE_REFERENCE_NAME = "footnoteReference";
/**
* Typographic glyph maps. DUPLICATED from `comment-anchor.ts` (the source of
* truth, `normalizeForMatch`) on purpose: those constants are private there and
* bound to that module's anchor-matching golden tests, so extracting them would
* risk changing anchor behaviour. Keeping a local copy makes this pass fully
* self-contained. If the anchor maps grow, mirror the change here.
*/
/** Typographic double-quote variants mapped to ASCII `"`. */
const DOUBLE_QUOTES = "«»„“”‟〝〞"";
/** Typographic single-quote/apostrophe variants mapped to ASCII `'`. */
const SINGLE_QUOTES = "‘’‚‛";
/** Dash variants mapped to ASCII `-`. */
const DASHES = "–—―−‐‑‒";
function cloneJson<T>(v: T): T {
if (typeof structuredClone === "function") return structuredClone(v);
return JSON.parse(JSON.stringify(v)) as T;
}
/**
* True for any character we collapse/replace with a single normal space.
* Mirrors `comment-anchor.ts`'s `isWhitespaceChar`: ASCII whitespace (`\s`
* covers tab/newline) plus the non-breaking / special spaces listed explicitly
* for determinism across engines.
*/
function isWhitespaceChar(ch: string): boolean {
return (
/\s/.test(ch) ||
ch === " " || // no-break space
ch === " " || // figure space
ch === " " || // narrow no-break space
ch === " " || // thin space
ch === " " || // hair space
ch === " " || // en space
ch === " " // em space
);
}
/**
* Map typographic quotes/dashes to ASCII and collapse every whitespace run
* (including NBSP & friends) to a SINGLE normal space. Does NOT trim — the
* whole-definition edge trim is applied separately so inter-node spacing across
* a multi-text-node definition is preserved.
*/
function normalizeAndCollapse(s: string): string {
let out = "";
let i = 0;
while (i < s.length) {
const ch = s[i];
if (isWhitespaceChar(ch)) {
while (i < s.length && isWhitespaceChar(s[i])) i++;
out += " ";
continue;
}
let mapped = ch;
if (DOUBLE_QUOTES.indexOf(ch) !== -1) mapped = '"';
else if (SINGLE_QUOTES.indexOf(ch) !== -1) mapped = "'";
else if (DASHES.indexOf(ch) !== -1) mapped = "-";
out += mapped;
i++;
}
return out;
}
/** Collect every text node inside `def`, in document order (deep). */
function collectTextNodes(node: any, out: any[]): void {
if (!node || typeof node !== "object") return;
if (node.type === "text" && typeof node.text === "string") out.push(node);
if (Array.isArray(node.content)) {
for (const child of node.content) collectTextNodes(child, out);
}
}
/** Collect every `footnoteDefinition` node in document order (deep). */
function collectDefinitions(node: any, out: any[]): void {
if (!node || typeof node !== "object") return;
if (node.type === FOOTNOTE_DEFINITION_NAME) out.push(node);
if (Array.isArray(node.content)) {
for (const child of node.content) collectDefinitions(child, out);
}
}
/**
* Normalize the text of one definition's text nodes IN PLACE: map glyphs +
* collapse whitespace on every node (marks untouched), then trim the leading
* edge of the first text node and the trailing edge of the last so the
* definition as a whole is trimmed WITHOUT dropping the spacing between two
* adjacent text nodes. The edge trims are guarded so an all-whitespace edge
* node is never emptied into a schema-invalid empty text node.
*/
function normalizeDefinitionText(def: any): void {
const textNodes: any[] = [];
collectTextNodes(def, textNodes);
for (const t of textNodes) {
// Skip text carrying a `code` mark: inline code is a verbatim literal, not
// prose typography. Rewriting quotes/dashes/special-spaces there would
// corrupt the literal's meaning (a string literal, an em-dash flag, i18n).
// Leaving it untouched also makes it contribute its RAW text to
// `footnoteMergeKey`, so two notes differing only by glyphs inside code
// stay distinct (while prose glyph-forks still merge). See #419.
if ((t.marks || []).some((m: any) => m?.type === "code")) continue;
t.text = normalizeAndCollapse(t.text);
}
if (textNodes.length === 0) return;
const hasCodeMark = (t: any): boolean =>
(t.marks || []).some((m: any) => m?.type === "code");
const first = textNodes[0];
if (!hasCodeMark(first)) {
const startTrimmed = first.text.replace(/^ +/, "");
if (startTrimmed !== "") first.text = startTrimmed;
}
const last = textNodes[textNodes.length - 1];
if (!hasCodeMark(last)) {
const endTrimmed = last.text.replace(/ +$/, "");
if (endTrimmed !== "") last.text = endTrimmed;
}
}
/** Rewrite `footnoteReference` ids IN PLACE using `defIdToCanon` (deep). */
function rehangReferences(
node: any,
defIdToCanon: Map<string, string>,
): void {
if (!node || typeof node !== "object") return;
if (node.type === FOOTNOTE_REFERENCE_NAME) {
const id = node?.attrs?.id;
if (typeof id === "string") {
const canon = defIdToCanon.get(id);
if (canon && canon !== id) node.attrs.id = canon;
}
}
if (Array.isArray(node.content)) {
for (const child of node.content) rehangReferences(child, defIdToCanon);
}
}
/**
* Stable, order-independent serialization of a mark's `attrs`: sort keys so the
* same attrs always yield the same string regardless of authoring order. Empty /
* missing attrs -> "" (so an attr-less mark keys identically to a type-only mark
* signature, preserving bold-vs-plain parity).
*/
function stableAttrs(attrs: any): string {
if (!attrs || typeof attrs !== "object") return "";
const sorted: Record<string, any> = {};
for (const k of Object.keys(attrs).sort()) sorted[k] = attrs[k];
return JSON.stringify(sorted);
}
/**
* ATTRS-AWARE merge key for a footnote definition. Deliberately DIVERGES from
* the shared `footnoteContentKey` (footnote-authoring.ts): that key's mark
* signature is TYPE-ONLY (`m.type`), so two definitions with identical visible
* text but marks differing only in ATTRIBUTES — most importantly a `link` with a
* different `href` (footnotes are usually citations/links), also `code` /
* `highlight` with differing attrs — collapse to the SAME key and get merged;
* one definition then loses its references and the canonicalizer deletes it as an
* orphan, silently dropping a distinct link target (data loss, #419).
*
* This key folds each mark's `attrs` (stable, sorted-key serialization) into the
* signature, so different-href / different-attr notes stay separate. We do NOT
* change `footnoteContentKey` itself: it is shared with the live
* `insertInlineFootnote` / `commentsToFootnotes` dedup and altering it there
* would change their behaviour — out of scope here.
*
* The TEXT portion mirrors `footnoteContentKey` exactly (per text node
* `text + mark-signature`, concatenated, whitespace-collapsed, trimmed) over the
* already-in-place-normalized text, so empty text still yields "" (empties never
* collapse) and merge parity with the rest of the pass is preserved.
*/
function footnoteMergeKey(defNode: any): string {
const parts: string[] = [];
const visit = (n: any): void => {
if (!n || typeof n !== "object") return;
if (n.type === "text" && typeof n.text === "string") {
const marks = Array.isArray(n.marks)
? n.marks
.filter((m: any) => m && m.type)
.map((m: any) => `${m.type}${stableAttrs(m.attrs)}`)
.sort()
.join(",")
: "";
parts.push(`${n.text}${marks}`);
}
if (Array.isArray(n.content)) for (const c of n.content) visit(c);
};
visit(defNode);
return parts
.join("")
.replace(/[ \t\r\n]+/g, " ")
.trim();
}
/**
* Normalize footnote-definition text and merge definitions whose normalized
* text (+ mark signature) matches. See the file header for the full contract.
* Pure (deep-clones input, deterministic, idempotent). Intended to run
* immediately BEFORE `canonicalizeFootnotes(doc)`.
*/
export function normalizeAndMergeFootnotes<T = any>(doc: T): T {
if (doc == null || typeof doc !== "object") return doc;
const out = cloneJson(doc) as any;
// 1) All definitions in document order; normalize each one's text in place.
const defNodes: any[] = [];
collectDefinitions(out, defNodes);
for (const def of defNodes) normalizeDefinitionText(def);
// 2) Merge key per definition (normalized text + inline-mark signature). The
// first definition in document order per key wins; later ones map onto it.
// Empty-text definitions (key === "") are NOT merged — otherwise every
// empty footnote would collapse into one (parity with insertInlineFootnote).
const keyToCanon = new Map<string, string>();
const defIdToCanon = new Map<string, string>();
for (const def of defNodes) {
const id = def?.attrs?.id;
if (typeof id !== "string" || id === "") continue;
const key = footnoteMergeKey(def);
if (key === "") continue;
const canon = keyToCanon.get(key);
if (canon === undefined) {
keyToCanon.set(key, id);
} else if (canon !== id) {
defIdToCanon.set(id, canon);
}
}
// 3) Re-hang references from duplicate ids onto the canonical id. Duplicate
// definitions keep their ids but now have no references -> the following
// canonicalizer pass drops them as orphans.
if (defIdToCanon.size > 0) rehangReferences(out, defIdToCanon);
return out;
}
+3
View File
@@ -14,6 +14,7 @@
* - `marks` arrays are preserved verbatim when fragments are split/reordered. * - `marks` arrays are preserved verbatim when fragments are split/reordered.
*/ */
import { normalizeAndMergeFootnotes } from "./footnote-normalize-merge.js";
import { import {
blockPlainText, blockPlainText,
footnoteContentKey, footnoteContentKey,
@@ -766,6 +767,8 @@ export function insertInlineFootnote(
appendDefinition(working, makeFootnoteDefinition(footnoteId, inline)); appendDefinition(working, makeFootnoteDefinition(footnoteId, inline));
} }
// #419: normalize + merge glyph-forked definitions before canonicalizing.
working = normalizeAndMergeFootnotes(working);
// Derive numbering + the single bottom list deterministically. // Derive numbering + the single bottom list deterministically.
working = canonicalizeFootnotes(working); working = canonicalizeFootnotes(working);
return { doc: working, inserted: true, footnoteId, reused }; return { doc: working, inserted: true, footnoteId, reused };
+12 -3
View File
@@ -236,7 +236,10 @@ export const SHARED_TOOL_SPECS = {
'heading {"type":"heading","attrs":{"level":2},"content":' + 'heading {"type":"heading","attrs":{"level":2},"content":' +
'[{"type":"text","text":"Title"}]}. Bold is a mark: ' + '[{"type":"text","text":"Title"}]}. Bold is a mark: ' +
'{"type":"text","text":"x","marks":[{"type":"bold"}]}. The node may be a ' + '{"type":"text","text":"x","marks":[{"type":"bold"}]}. The node may be a ' +
'JSON object or a JSON string (both accepted). Cheaper and safer than ' + 'JSON object or a JSON string (both accepted). EVERY node, including ' +
'nested children, must carry a string `type` from the Docmost schema; ' +
'text leaves are {"type":"text","text":"..."} (a bare {"text":"..."} is ' +
'rejected up front). Cheaper and safer than ' +
'replacing the whole document for one-block structural edits. Reversible: ' + 'replacing the whole document for one-block structural edits. Reversible: ' +
'the previous version is kept in page history.', 'the previous version is kept in page history.',
tier: 'deferred', tier: 'deferred',
@@ -282,7 +285,10 @@ export const SHARED_TOOL_SPECS = {
'{"type":"paragraph","content":[{"type":"text","text":"Hello"}]} or a ' + '{"type":"paragraph","content":[{"type":"text","text":"Hello"}]} or a ' +
'heading {"type":"heading","attrs":{"level":2},"content":' + 'heading {"type":"heading","attrs":{"level":2},"content":' +
'[{"type":"text","text":"Title"}]}. Bold is a mark: ' + '[{"type":"text","text":"Title"}]}. Bold is a mark: ' +
'{"type":"text","text":"x","marks":[{"type":"bold"}]}. The node may be a ' + '{"type":"text","text":"x","marks":[{"type":"bold"}]}. EVERY node, ' +
'including nested children, must carry a string `type` from the Docmost ' +
'schema; text leaves are {"type":"text","text":"..."} (a bare ' +
'{"text":"..."} is rejected up front). The node may be a ' +
'JSON object or a JSON string (both accepted). Reversible via page history.', 'JSON object or a JSON string (both accepted). Reversible via page history.',
tier: 'deferred', tier: 'deferred',
catalogLine: catalogLine:
@@ -705,7 +711,10 @@ export const SHARED_TOOL_SPECS = {
'"paragraph","content":[{"type":"text","text":"Hi"}]}]}. `content` may be ' + '"paragraph","content":[{"type":"text","text":"Hi"}]}]}. `content` may be ' +
'a JSON object or a JSON string (both accepted), and is OPTIONAL: omit it ' + 'a JSON object or a JSON string (both accepted), and is OPTIONAL: omit it ' +
'to update only the title (though prefer the rename-page tool for a title-only ' + 'to update only the title (though prefer the rename-page tool for a title-only ' +
'change). Supplying neither content nor title is an error. Reversible: ' + 'change). Supplying neither content nor title is an error. EVERY node, ' +
'including nested children, must carry a string `type` from the Docmost ' +
'schema; text leaves are {"type":"text","text":"..."} (a bare ' +
'{"text":"..."} is rejected up front). Reversible: ' +
'the previous version is kept in page history.', 'the previous version is kept in page history.',
tier: 'deferred', tier: 'deferred',
catalogLine: catalogLine:
@@ -0,0 +1,240 @@
// Mock regression for the FAIL-FAST invalid-node validation (#409).
//
// A structural editor (patch_node / insert_node / update_page_json) given a doc
// whose NESTED child has an absent/unknown `type` (the exact shape the Yjs
// encoder rejects with `Unknown node type: undefined`) must throw a RICH,
// path-anchored error BEFORE it ever opens a collab session or takes a page
// lock. We prove the fail-fast by standing up a collab stack whose HTTP handler
// records EVERY request: a correct fail-fast never even fetches the collab
// token (which `getCollabTokenWithReauth`, called AFTER the validation, would
// request), and never drives a document change on the Hocuspocus doc.
//
// The happy path (a well-formed doc) is exercised too: it must reach the collab
// write and succeed, so the gate is not over-eager.
//
// findInvalidNode's per-shape summaries are unit-tested in the package
// (test/find-invalid-node.test.ts); this exercises the END-TO-END wiring through
// the real client methods.
import { test, after } from "node:test";
import assert from "node:assert/strict";
import http from "node:http";
import { WebSocketServer } from "ws";
import { Hocuspocus } from "@hocuspocus/server";
import { DocmostClient } from "../../build/client.js";
import { buildYDoc } from "../../build/lib/collaboration.js";
// A minimal valid seed doc with a real block id, so the happy-path patch_node
// finds its target.
const SEED_ID = "seed-para-id";
function seedDoc() {
return {
type: "doc",
content: [
{
type: "paragraph",
attrs: { id: SEED_ID },
content: [{ type: "text", text: "seed" }],
},
],
};
}
// Stand up an HTTP server that authenticates + hands out a collab token AND
// upgrades /collab to a Hocuspocus instance seeded with the doc. `state` records
// whether the collab token was ever fetched (proving the write path was entered)
// and whether the Hocuspocus doc ever changed.
async function spawnCollabStack() {
const state = { changed: false, collabTokenFetched: false };
const hocuspocus = new Hocuspocus({
quiet: true,
async onLoadDocument() {
return buildYDoc(seedDoc());
},
async onChange() {
state.changed = true;
},
});
const wss = new WebSocketServer({ noServer: true });
const server = http.createServer((req, res) => {
let raw = "";
req.on("data", (c) => (raw += c));
req.on("end", () => {
if (req.url === "/api/auth/login") {
res.writeHead(200, {
"Content-Type": "application/json",
"Set-Cookie": "authToken=t; Path=/; HttpOnly",
});
res.end(JSON.stringify({ success: true }));
return;
}
if (req.url === "/api/auth/collab-token") {
state.collabTokenFetched = true;
res.writeHead(200, { "Content-Type": "application/json" });
res.end(JSON.stringify({ data: { token: "collab-jwt" } }));
return;
}
res.writeHead(404, { "Content-Type": "application/json" });
res.end(JSON.stringify({ message: "not found" }));
});
});
server.on("upgrade", (request, socket, head) => {
if (!request.url || !request.url.startsWith("/collab")) {
socket.destroy();
return;
}
wss.handleUpgrade(request, socket, head, (ws) => {
hocuspocus.handleConnection(ws, request);
});
});
const baseURL = await new Promise((resolve) => {
server.listen(0, "127.0.0.1", () => {
const { port } = server.address();
resolve(`http://127.0.0.1:${port}/api`);
});
});
openStacks.push({ server, hocuspocus });
return { state, baseURL };
}
const openStacks = [];
after(async () => {
await Promise.all(
openStacks.map(
({ server, hocuspocus }) =>
new Promise((resolve) => {
server.close(() => {
Promise.resolve(hocuspocus.destroy?.()).finally(resolve);
});
}),
),
);
});
const PAGE = "11111111-1111-4111-8111-111111111111";
// A node whose NESTED text leaf is missing "type":"text" (dominant #409 shape).
const nestedTypelessNode = () => ({
type: "paragraph",
content: [{ text: "oops", marks: [] }],
});
// A node with a NESTED unknown type NAME (typo).
const nestedUnknownTypeNode = () => ({
type: "paragraph",
content: [{ type: "paragraf", content: [{ type: "text", text: "x" }] }],
});
test("patch_node fails fast on a nested typeless node — no collab connection", async () => {
const { state, baseURL } = await spawnCollabStack();
const client = new DocmostClient(baseURL, "user@example.com", "pw");
await assert.rejects(
() => client.patchNode(PAGE, SEED_ID, nestedTypelessNode()),
(err) => {
assert.match(err.message, /patch_node: invalid node/);
assert.match(err.message, /missing "type"/);
assert.match(err.message, /content\[0\]/); // path-anchored
return true;
},
);
assert.equal(
state.collabTokenFetched,
false,
"must NOT fetch a collab token — validation runs before getCollabTokenWithReauth",
);
assert.equal(state.changed, false, "the collab doc must never be written");
});
test("insert_node fails fast on a nested UNKNOWN type — no collab connection", async () => {
const { state, baseURL } = await spawnCollabStack();
const client = new DocmostClient(baseURL, "user@example.com", "pw");
await assert.rejects(
() =>
client.insertNode(PAGE, nestedUnknownTypeNode(), {
position: "append",
}),
(err) => {
assert.match(err.message, /insert_node: invalid node/);
assert.match(err.message, /unknown node type "paragraf"/);
return true;
},
);
assert.equal(state.collabTokenFetched, false);
assert.equal(state.changed, false);
});
test("update_page_json fails fast on a nested typeless node — no collab connection", async () => {
const { state, baseURL } = await spawnCollabStack();
const client = new DocmostClient(baseURL, "user@example.com", "pw");
const badDoc = {
type: "doc",
content: [{ type: "paragraph", content: [{ text: "oops" }] }],
};
await assert.rejects(
() => client.updatePageJson(PAGE, badDoc),
(err) => {
// update_page_json runs validateDocStructure first (string-type check),
// which already rejects a typeless node — so the message may come from
// either guard, but the write must not happen.
assert.match(err.message, /type/i);
return true;
},
);
assert.equal(state.collabTokenFetched, false);
assert.equal(state.changed, false);
});
test("update_page_json fails fast on a nested UNKNOWN type name — rich #409 message", async () => {
const { state, baseURL } = await spawnCollabStack();
const client = new DocmostClient(baseURL, "user@example.com", "pw");
// validateDocStructure passes (type is a string); assertValidNodeShape must
// catch the unknown schema name and produce the rich path-anchored message.
const badDoc = {
type: "doc",
content: [{ type: "paragraf", content: [{ type: "text", text: "x" }] }],
};
await assert.rejects(
() => client.updatePageJson(PAGE, badDoc),
(err) => {
assert.match(err.message, /update_page_json: invalid node/);
assert.match(err.message, /unknown node type "paragraf"/);
return true;
},
);
assert.equal(state.collabTokenFetched, false);
assert.equal(state.changed, false);
});
test("patch_node with a well-formed node proceeds to the collab write", async () => {
const { state, baseURL } = await spawnCollabStack();
const client = new DocmostClient(baseURL, "user@example.com", "pw");
const result = await client.patchNode(PAGE, SEED_ID, {
type: "paragraph",
content: [{ type: "text", text: "replacement" }],
});
assert.equal(result.success, true);
assert.equal(result.replaced, 1);
assert.equal(
state.collabTokenFetched,
true,
"a valid node must reach the collab write path",
);
assert.equal(state.changed, true, "the collab doc must be written");
});
@@ -0,0 +1,317 @@
import { test } from "node:test";
import assert from "node:assert/strict";
import { normalizeAndMergeFootnotes } from "../../build/lib/footnote-normalize-merge.js";
import { canonicalizeFootnotes } from "../../build/lib/footnote-canonicalize.js";
function findAll(node, type, acc = []) {
if (!node || typeof node !== "object") return acc;
if (node.type === type) acc.push(node);
if (Array.isArray(node.content)) {
for (const c of node.content) findAll(c, type, acc);
}
return acc;
}
const defs = (doc) => findAll(doc, "footnoteDefinition");
const defIds = (doc) => defs(doc).map((d) => d.attrs.id);
const refIds = (doc) => findAll(doc, "footnoteReference").map((r) => r.attrs.id);
const defText = (d) =>
findAll(d, "text")
.map((t) => t.text)
.join("");
const ref = (id) => ({ type: "footnoteReference", attrs: { id } });
const para = (...inline) => ({ type: "paragraph", content: inline });
const txt = (text, marks) =>
marks ? { type: "text", text, marks } : { type: "text", text };
const def = (id, ...inline) => ({
type: "footnoteDefinition",
attrs: { id },
content: [para(...inline)],
});
const list = (...defs) => ({ type: "footnotesList", content: defs });
const doc = (...content) => ({ type: "doc", content });
// --- Normalization + merge of glyph forks ----------------------------------
test("typographic double quotes «…» vs \"…\" merge into one", () => {
const d = doc(
para(txt("a"), ref("A"), txt(" b"), ref("B")),
list(def("A", txt("«word»")), def("B", txt('"word"'))),
);
const out = normalizeAndMergeFootnotes(d);
// Both references now point at the first definition's id.
assert.deepEqual(refIds(out), ["A", "A"]);
// Surviving text is ASCII-normalized.
assert.equal(defText(defs(out)[0]), '"word"');
// Duplicate def kept its id (canonicalizer removes it as an orphan later).
const canon = canonicalizeFootnotes(out);
assert.deepEqual(defIds(canon), ["A"]);
assert.equal(findAll(canon, "footnotesList").length, 1);
});
test("em/en dash and hyphen merge", () => {
const d = doc(
para(ref("A"), ref("B"), ref("C")),
list(
def("A", txt("see — here")),
def("B", txt("see – here")),
def("C", txt("see - here")),
),
);
const out = normalizeAndMergeFootnotes(d);
assert.deepEqual(refIds(out), ["A", "A", "A"]);
assert.equal(defText(defs(out)[0]), "see - here");
});
test("NBSP and extra spaces merge with normal spacing", () => {
const d = doc(
para(ref("A"), ref("B")),
list(
def("A", txt("foo bar")), // NBSP
def("B", txt("foo bar")), // collapsed spaces
),
);
const out = normalizeAndMergeFootnotes(d);
assert.deepEqual(refIds(out), ["A", "A"]);
assert.equal(defText(defs(out)[0]), "foo bar");
});
test("same text but different styling (bold vs plain) does NOT merge", () => {
const d = doc(
para(ref("A"), ref("B")),
list(
def("A", txt("word", [{ type: "bold" }])),
def("B", txt("word")),
),
);
const out = normalizeAndMergeFootnotes(d);
// No re-hang: references keep their own ids.
assert.deepEqual(refIds(out), ["A", "B"]);
assert.deepEqual(defIds(out), ["A", "B"]);
// Marks preserved on the surviving text node.
assert.deepEqual(defs(out)[0].content[0].content[0].marks, [
{ type: "bold" },
]);
});
test("same text but a link mark with different href does NOT merge (data-loss guard)", () => {
const d = doc(
para(ref("A"), ref("B")),
list(
def("A", txt("source", [{ type: "link", attrs: { href: "https://a.example/1" } }])),
def("B", txt("source", [{ type: "link", attrs: { href: "https://b.example/2" } }])),
),
);
const out = normalizeAndMergeFootnotes(d);
// No re-hang: each reference keeps its own definition (distinct link target).
assert.deepEqual(refIds(out), ["A", "B"]);
assert.deepEqual(defIds(out), ["A", "B"]);
// Both distinct hrefs survive.
assert.deepEqual(
defs(out).map((dn) => dn.content[0].content[0].marks[0].attrs.href),
["https://a.example/1", "https://b.example/2"],
);
// Canonicalize keeps both as two tail entries (neither is an orphan).
const canon = canonicalizeFootnotes(out);
assert.deepEqual(defIds(canon), ["A", "B"]);
assert.deepEqual(refIds(canon), ["A", "B"]);
});
test("same text and SAME link href still merges (attrs-aware key doesn't over-separate)", () => {
const d = doc(
para(ref("A"), ref("B")),
list(
def("A", txt("source", [{ type: "link", attrs: { href: "https://a.example/1" } }])),
def("B", txt("source", [{ type: "link", attrs: { href: "https://a.example/1" } }])),
),
);
const out = normalizeAndMergeFootnotes(d);
assert.deepEqual(refIds(out), ["A", "A"]);
const canon = canonicalizeFootnotes(out);
assert.deepEqual(defIds(canon), ["A"]);
assert.equal(findAll(canon, "footnotesList").length, 1);
});
test("marks are kept on merged (surviving) definition text", () => {
const d = doc(
para(ref("A"), ref("B")),
list(
def("A", txt("«x»", [{ type: "italic" }])),
def("B", txt("«x»", [{ type: "italic" }])),
),
);
const out = normalizeAndMergeFootnotes(d);
assert.deepEqual(refIds(out), ["A", "A"]);
assert.deepEqual(defs(out)[0].content[0].content[0].marks, [
{ type: "italic" },
]);
assert.equal(defText(defs(out)[0]), '"x"');
});
// --- Inline code is verbatim (not typography) ------------------------------
test("text inside a code mark is left verbatim; prose in the same def is normalized", () => {
const d = doc(
para(ref("A")),
list({
type: "footnoteDefinition",
attrs: { id: "A" },
content: [
para(
txt("a—b «x»", [{ type: "code" }]),
txt(" prose «y» — z"),
),
],
}),
);
const out = normalizeAndMergeFootnotes(d);
const nodes = findAll(defs(out)[0], "text");
// Code node: byte-for-byte unchanged (typography preserved).
assert.equal(nodes[0].text, "a—b «x»");
// Prose node: dashes/quotes normalized to ASCII.
assert.equal(nodes[1].text, ' prose "y" - z');
});
test("two notes differing ONLY by glyphs inside a code mark do NOT merge", () => {
const d = doc(
para(ref("A"), ref("B")),
list(
def("A", txt("«x»", [{ type: "code" }]), txt(" same prose «q»")),
def("B", txt('"x"', [{ type: "code" }]), txt(" same prose «q»")),
),
);
const out = normalizeAndMergeFootnotes(d);
// Prose is identical after normalization, but the code literals differ raw
// -> the merge key diverges -> both definitions survive, no re-hang.
assert.deepEqual(refIds(out), ["A", "B"]);
assert.deepEqual(defIds(out), ["A", "B"]);
// Each code literal stays verbatim.
assert.equal(defs(out)[0].content[0].content[0].text, "«x»");
assert.equal(defs(out)[1].content[0].content[0].text, '"x"');
// Both survive canonicalization (neither is an orphan).
const canon = canonicalizeFootnotes(out);
assert.deepEqual(defIds(canon), ["A", "B"]);
assert.deepEqual(refIds(canon), ["A", "B"]);
});
// --- Composition with the canonicalizer ------------------------------------
test("pass + canonicalize: single tail list and sequential numbering", () => {
const d = doc(
para(txt("intro "), ref("A"), txt(" middle "), ref("B")),
list(def("A", txt("«note»")), def("B", txt('"note"'))),
);
const canon = canonicalizeFootnotes(normalizeAndMergeFootnotes(d));
assert.equal(findAll(canon, "footnotesList").length, 1);
assert.deepEqual(defIds(canon), ["A"]);
assert.deepEqual(refIds(canon), ["A", "A"]);
});
// --- Idempotency -----------------------------------------------------------
test("idempotent: a second run is a no-op", () => {
const d = doc(
para(ref("A"), ref("B")),
list(def("A", txt("«word»")), def("B", txt('"word"'))),
);
const once = normalizeAndMergeFootnotes(d);
const twice = normalizeAndMergeFootnotes(once);
assert.deepEqual(twice, once);
});
test("input document is not mutated (pure)", () => {
const d = doc(
para(ref("A"), ref("B")),
list(def("A", txt("«word»")), def("B", txt('"word"'))),
);
const snapshot = JSON.parse(JSON.stringify(d));
normalizeAndMergeFootnotes(d);
assert.deepEqual(d, snapshot);
});
// --- Nested definitions ----------------------------------------------------
test("definitions nested in a callout are normalized and merged", () => {
const callout = (...content) => ({
type: "callout",
attrs: { type: "info" },
content,
});
const d = doc(
para(ref("A"), ref("B")),
callout(list(def("A", txt("«c»")), def("B", txt('"c"')))),
);
const out = normalizeAndMergeFootnotes(d);
assert.deepEqual(refIds(out), ["A", "A"]);
assert.equal(defText(defs(out)[0]), '"c"');
});
// --- Empty footnotes -------------------------------------------------------
test("empty footnotes do NOT collapse into each other", () => {
const d = doc(
para(ref("A"), ref("B")),
list(def("A", txt("")), { type: "footnoteDefinition", attrs: { id: "B" }, content: [{ type: "paragraph" }] }),
);
const out = normalizeAndMergeFootnotes(d);
// Both empty definitions keep distinct ids; references unchanged.
assert.deepEqual(refIds(out), ["A", "B"]);
assert.deepEqual(defIds(out), ["A", "B"]);
});
// --- Body text left untouched ----------------------------------------------
test("body text (outside footnotes) is NOT normalized", () => {
const d = doc(
para(txt("body «quoted» — dash"), ref("A")),
list(def("A", txt("«note»"))),
);
const out = normalizeAndMergeFootnotes(d);
// Body paragraph keeps its typographic glyphs verbatim.
assert.equal(out.content[0].content[0].text, "body «quoted» — dash");
// Footnote text IS normalized.
assert.equal(defText(defs(out)[0]), '"note"');
});
// --- Multi-paragraph structure preserved -----------------------------------
test("multi-paragraph definition: text normalized, structure preserved", () => {
const d = doc(
para(ref("A")),
list({
type: "footnoteDefinition",
attrs: { id: "A" },
content: [para(txt("«p1»")), para(txt("p2 — end"))],
}),
);
const out = normalizeAndMergeFootnotes(d);
const def0 = defs(out)[0];
assert.equal(def0.content.length, 2);
assert.equal(def0.content[0].content[0].text, '"p1"');
assert.equal(def0.content[1].content[0].text, "p2 - end");
});
// --- Multi-reference footnote not broken -----------------------------------
test("one id shared by multiple references is preserved", () => {
const d = doc(
para(ref("A"), txt(" x "), ref("A")),
list(def("A", txt("note"))),
);
const out = normalizeAndMergeFootnotes(d);
assert.deepEqual(refIds(out), ["A", "A"]);
assert.deepEqual(defIds(out), ["A"]);
});
// --- Whole-definition edge trim --------------------------------------------
test("leading/trailing whitespace is trimmed for the merge and stored text", () => {
const d = doc(
para(ref("A"), ref("B")),
list(def("A", txt(" hello ")), def("B", txt("hello"))),
);
const out = normalizeAndMergeFootnotes(d);
assert.deepEqual(refIds(out), ["A", "A"]);
assert.equal(defText(defs(out)[0]), "hello");
});
@@ -0,0 +1,82 @@
// #409: unstorableYjsError diagnostics PRECEDENCE. The opaque Yjs encode failure
// (`Unknown node type: undefined`) is a node-SHAPE problem, so the shared
// findInvalidNode is consulted FIRST and yields a path-anchored node message;
// only a shape-sound doc falls back to findUnstorableAttr (undefined/function/
// etc. attr values). Exercised through `assertYjsEncodable`, which runs the same
// encode + error-wrapping the live write path uses.
import { test } from "node:test";
import assert from "node:assert/strict";
import { assertYjsEncodable } from "../../build/lib/collaboration.js";
import {
findInvalidNode,
findUnstorableAttr,
} from "@docmost/prosemirror-markdown";
const doc = (...content) => ({ type: "doc", content });
test("a nested typeless node yields the rich node-shape message (not an attr hint)", () => {
const bad = doc({
type: "paragraph",
content: [{ text: "oops", marks: [] }], // missing "type":"text"
});
assert.throws(
() => assertYjsEncodable(bad),
(err) => {
assert.match(err.message, /Invalid node:/);
assert.match(err.message, /missing "type"/);
assert.match(err.message, /content\[0\]/);
// It must NOT fall through to the generic attribute sentence.
assert.doesNotMatch(err.message, /Offending attribute/);
assert.doesNotMatch(
err.message,
/attribute likely holds a value Yjs cannot store/,
);
return true;
},
);
});
test("PRECEDENCE: a genuine undefined-attr case is a node-SHAPE-clean case, so the attr fallback fires", () => {
// A doc whose node shapes are ALL valid but that carries a Yjs-unstorable
// undefined attribute. unstorableYjsError checks findInvalidNode FIRST (must
// miss here) and only then findUnstorableAttr (must hit) — this is exactly the
// division of labor that keeps a real attr problem from being mislabelled as a
// node-shape problem, and vice versa. We assert the two helpers directly (the
// wrapper is not exported) because sanitizeForYjs strips undefined attrs before
// the live encoder ever sees them, so this branch cannot be reached through
// assertYjsEncodable without also failing the clone.
const attrProblem = doc({
type: "paragraph",
attrs: { id: "p1", indent: undefined },
content: [{ type: "text", text: "hi" }],
});
// findInvalidNode: shape is clean -> null (so the wrapper does NOT emit
// "Invalid node").
assert.equal(findInvalidNode(attrProblem), null);
// findUnstorableAttr: pinpoints the undefined attr -> the fallback message.
assert.match(findUnstorableAttr(attrProblem) ?? "", /indent \(undefined\)/);
});
test("PRECEDENCE: a node-shape problem is caught by findInvalidNode even when an attr is also unstorable", () => {
// Both a shape problem (typeless nested leaf) AND an unstorable attr exist;
// findInvalidNode wins, so the model is pointed at the node shape (the real
// root cause of `Unknown node type: undefined`), not the attribute.
const both = doc({
type: "paragraph",
attrs: { id: "p1", indent: undefined },
content: [{ text: "oops" }],
});
const shape = findInvalidNode(both);
assert.notEqual(shape, null);
assert.match(shape.summary, /missing "type"/);
});
test("a fully valid document encodes without throwing", () => {
const good = doc({
type: "paragraph",
attrs: { id: "p1" },
content: [{ type: "text", text: "hi" }],
});
assert.doesNotThrow(() => assertYjsEncodable(good));
});
@@ -56,6 +56,7 @@ export {
deleteNodeById, deleteNodeById,
sanitizeForYjs, sanitizeForYjs,
findUnstorableAttr, findUnstorableAttr,
findInvalidNode,
insertNodeRelative, insertNodeRelative,
readTable, readTable,
insertTableRow, insertTableRow,
@@ -14,7 +14,9 @@
* `content`, non-object nodes, and absent `attrs` are tolerated. * `content`, non-object nodes, and absent `attrs` are tolerated.
*/ */
import { getSchema } from "@tiptap/core";
import { stripInlineMarkdown } from "./text-normalize.js"; import { stripInlineMarkdown } from "./text-normalize.js";
import { docmostExtensions } from "./docmost-schema.js";
/** Deep-clone a JSON-serializable value without mutating the original. */ /** Deep-clone a JSON-serializable value without mutating the original. */
function clone<T>(value: T): T { function clone<T>(value: T): T {
@@ -383,6 +385,119 @@ export function findUnstorableAttr(doc: any): string | null {
return null; return null;
} }
/**
* The Docmost schema's known node and mark NAME sets, derived ONCE from the very
* same `docmostExtensions` the Yjs encode path builds its schema from
* (`getSchema(docmostExtensions)` — mirrored in mcp's `docmostSchema`). Deriving
* both from the same extension list guarantees `findInvalidNode`'s "known type"
* set matches exactly what `PMNode.fromJSON`/`toYdoc` will actually accept, so
* the walker never flags a node the encoder would have stored (or vice versa).
* Lazy + cached: the schema is only built on first use.
*/
let schemaNames: { nodes: Set<string>; marks: Set<string> } | null = null;
function getSchemaNames(): { nodes: Set<string>; marks: Set<string> } {
if (schemaNames == null) {
const schema = getSchema(docmostExtensions);
schemaNames = {
nodes: new Set(Object.keys(schema.nodes)),
marks: new Set(Object.keys(schema.marks)),
};
}
return schemaNames;
}
/**
* Depth-first walk of the JSON `content` tree looking for the FIRST node whose
* SHAPE the Yjs encode path will reject with an opaque
* `Unknown node type: undefined` (issue #409). Returns `{ path, summary }` for
* the offending node, or `null` when every node (and every mark) is a known
* Docmost schema type.
*
* Two failure modes are detected, in order, per node:
* 1. `type` is missing or not a string — the dominant `undefined` case, e.g.
* a text leaf written as `{"text":"foo"}` with no `"type":"text"`.
* 2. `type` is a string but NOT a known Docmost node name (a typo / unknown
* block), OR one of the node's marks carries an unknown mark name.
*
* The returned `summary` is a model-actionable, path-anchored message such as:
* `node.content[2].content[0]: missing "type" (keys: text, marks) — did you
* mean {"type": "text", ...}?`
* or for an unknown type:
* `node.content[1]: unknown node type "paragraf" — not in the Docmost schema`
*
* `path` is the same dotted JSON path used in the summary (e.g.
* `node.content[2].content[0]`) so callers can surface it separately. Null-safe:
* a non-object doc returns `null`.
*
* NOTE: This is a SHAPE check, not a full ProseMirror content-model validation
* (it does not verify that a paragraph may legally contain a table, etc.). Its
* job is to turn the specific "unknown/absent node type" Yjs crash into a clear,
* pre-write diagnostic; the schema's own `.check()` still catches deeper
* content-model violations at encode time.
*/
export function findInvalidNode(
doc: any,
): { path: string; summary: string } | null {
if (!isObject(doc)) return null;
const { nodes, marks } = getSchemaNames();
// Build the "did you mean" hint for a typeless node from its own keys, so the
// model sees WHICH object is malformed and the canonical text-leaf fix.
const keyHint = (node: Record<string, any>): string => {
const keys = Object.keys(node);
const looksLikeText =
typeof node.text === "string" && node.type === undefined;
const suffix = looksLikeText
? ` — did you mean {"type": "text", ...}?`
: ` — every node needs a string "type" from the Docmost schema`;
return `missing "type" (keys: ${keys.join(", ") || "none"})${suffix}`;
};
const walk = (
node: any,
path: string,
): { path: string; summary: string } | null => {
if (!isObject(node)) return null;
// (1) missing / non-string type.
if (typeof node.type !== "string") {
return { path, summary: `${path}: ${keyHint(node)}` };
}
// (2) string type that is not a known Docmost node.
if (!nodes.has(node.type)) {
return {
path,
summary: `${path}: unknown node type "${node.type}" — not in the Docmost schema`,
};
}
// (2b) unknown mark on an otherwise-valid node.
if (Array.isArray(node.marks)) {
for (let i = 0; i < node.marks.length; i++) {
const mark = node.marks[i];
if (isObject(mark) && typeof mark.type === "string" && !marks.has(mark.type)) {
return {
path: `${path}.marks[${i}]`,
summary: `${path}.marks[${i}]: unknown mark type "${mark.type}" — not in the Docmost schema`,
};
}
}
}
if (Array.isArray(node.content)) {
for (let i = 0; i < node.content.length; i++) {
const hit = walk(node.content[i], `${path}.content[${i}]`);
if (hit != null) return hit;
}
}
return null;
};
// The root doc node is addressed as "node" (matching the mcp arg name); its
// children are node.content[i]. The root itself is checked too so a typeless
// root is reported rather than silently skipped.
return walk(doc, "node");
}
/** /**
* Table structural node types and the container each must live directly inside. * Table structural node types and the container each must live directly inside.
* Used by `insertNodeRelative` to splice rows/cells into the correct ancestor * Used by `insertNodeRelative` to splice rows/cells into the correct ancestor
@@ -0,0 +1,102 @@
import { describe, expect, it } from 'vitest';
import { findInvalidNode } from '../src/lib/node-ops.js';
// findInvalidNode (#409): a depth-first SHAPE gate that turns the encoder's
// opaque `Unknown node type: undefined` into a path-anchored, pre-write
// diagnostic. It flags the FIRST node whose `type` is absent/non-string or not
// a known Docmost schema node, or that carries an unknown mark; returns null for
// a well-formed doc.
const doc = (...content: any[]) => ({ type: 'doc', content });
const para = (...content: any[]) => ({
type: 'paragraph',
attrs: { id: 'p1' },
content,
});
const text = (value: string, marks?: any[]) => {
const node: any = { type: 'text', text: value };
if (marks) node.marks = marks;
return node;
};
describe('findInvalidNode', () => {
it('returns null for a fully valid document', () => {
const good = doc(
para(text('hello ', [{ type: 'bold' }]), text('world')),
{
type: 'heading',
attrs: { id: 'h1', level: 2 },
content: [text('Title')],
},
);
expect(findInvalidNode(good)).toBeNull();
});
it('flags a NESTED typeless text leaf with a path-precise summary', () => {
// A text leaf written as {"text":"foo"} with no "type":"text" — the dominant
// `Unknown node type: undefined` cause.
const bad = doc(
para(text('ok')),
para({ text: 'foo', marks: [] } as any),
);
const hit = findInvalidNode(bad);
expect(hit).not.toBeNull();
// Second paragraph (index 1), first child (index 0).
expect(hit!.path).toBe('node.content[1].content[0]');
expect(hit!.summary).toContain('node.content[1].content[0]');
expect(hit!.summary).toContain('missing "type"');
expect(hit!.summary).toContain('keys: text, marks');
expect(hit!.summary).toContain('did you mean {"type": "text", ...}');
});
it('flags a non-string type (e.g. numeric)', () => {
const bad = doc(para({ type: 123, content: [] } as any));
const hit = findInvalidNode(bad);
expect(hit).not.toBeNull();
expect(hit!.path).toBe('node.content[0].content[0]');
expect(hit!.summary).toContain('missing "type"');
});
it('flags an UNKNOWN node type name that is not in the Docmost schema', () => {
const bad = doc({
type: 'paragraf', // typo — not a real node
attrs: { id: 'x' },
content: [text('hi')],
});
const hit = findInvalidNode(bad);
expect(hit).not.toBeNull();
expect(hit!.path).toBe('node.content[0]');
expect(hit!.summary).toContain('unknown node type "paragraf"');
expect(hit!.summary).toContain('not in the Docmost schema');
});
it('flags an UNKNOWN mark type on an otherwise-valid node', () => {
const bad = doc(para(text('hi', [{ type: 'blink' }])));
const hit = findInvalidNode(bad);
expect(hit).not.toBeNull();
expect(hit!.path).toBe('node.content[0].content[0].marks[0]');
expect(hit!.summary).toContain('unknown mark type "blink"');
});
it('accepts every real Docmost node/mark type it is asked about', () => {
// Known node (callout) and known marks (italic, code) must NOT be flagged.
const good = doc({
type: 'callout',
attrs: { id: 'c1', type: 'info' },
content: [para(text('x', [{ type: 'italic' }, { type: 'code' }]))],
});
expect(findInvalidNode(good)).toBeNull();
});
it('reports the root itself when the root is typeless', () => {
const hit = findInvalidNode({ content: [] } as any);
expect(hit).not.toBeNull();
expect(hit!.path).toBe('node');
});
it('is null-safe for non-object input', () => {
expect(findInvalidNode(null)).toBeNull();
expect(findInvalidNode(undefined)).toBeNull();
expect(findInvalidNode('nope' as any)).toBeNull();
});
});