diff --git a/CHANGELOG.md b/CHANGELOG.md index a46c61b8..840f7cda 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `AI_AGENT_ROLES_CATALOG_URL` env var — an `http(s)://` base URL to the catalog's raw files; the image ships a per-branch default baked in CI, and it can be overridden at runtime via the env var (see `.env.example`). (#222) +- **Author footnotes inline from an agent, and deterministic server-side footnote + canonicalization on every non-editor write path.** A new MCP `insert_footnote` + tool places a footnote at a body anchor by content only — the agent supplies + WHERE (anchor text) and WHAT (markdown); the number and the bottom + `footnotesList` are derived server-side, so an agent can never assign a number, + edit the list, or desync, and a same-content note reuses one definition. Under + the hood, the editor's footnote-integrity invariant (one trailing list, + numbering by first reference, no orphans/duplicates, no raw `[^id]`) is now + enforced as a pure `canonicalizeFootnotes(doc)` on the write paths that bypass + the editor's plugins: server markdown/HTML import, `PageService` create and + full-document (`replace`) updates, the client markdown paste, and the MCP + `markdownToProseMirror` / `update_page_json` / `docmost_transform` / + `insert_footnote` paths. It is idempotent (a no-op once canonical) and is + deliberately NOT applied to append/prepend fragments. (#228) ### Fixed diff --git a/README.ru.md b/README.ru.md index 132ba442..ca980d31 100644 --- a/README.ru.md +++ b/README.ru.md @@ -33,7 +33,7 @@ | --- | --- | | **Удалён EE-код** | Вырезан весь код Enterprise-редакции на клиенте и сервере; это чистая community/AGPL-сборка без лицензионных проверок. | | **Резолв комментариев** | Переписан с нуля как community-функция (резолв / переоткрытие с вкладками «Открытые» / «Решённые»). EE-код не используется, доступно любому, кто может комментировать. | -| **Встроенный MCP-сервер** | Community MCP-сервер (`@docmost/mcp`, 38 инструментов) отдаётся по HTTP на `/mcp` — без enterprise-лицензии. Заменяет удалённый лицензируемый EE MCP. | +| **Встроенный MCP-сервер** | Community MCP-сервер (`@docmost/mcp`, 39 инструментов) отдаётся по HTTP на `/mcp` — без enterprise-лицензии. Заменяет удалённый лицензируемый EE MCP. | | **Чат с AI-агентом** | Встроенный чат с AI-агентом по содержимому вики, написанный с нуля как community-функция — без enterprise-лицензии. Агент читает и редактирует страницы от вашего имени (в рамках ваших прав), с полнотекстовым + векторным (RAG) поиском и опциональным доступом в интернет через внешние MCP-серверы. | | **Ребрендинг** | Логотип / название приложения изменены с *Docmost* на *Gitmost*. | | **Компактное дерево страниц** | Отступ дерева страниц по умолчанию уменьшен с 16px до 8px на уровень вложенности. | @@ -44,7 +44,7 @@ В Gitmost есть **наш собственный MCP-сервер** — [docmost-mcp](https://github.com/vvzvlad/docmost-mcp), который мы написали сами, — **встроенный прямо в приложение** и доступный на `/mcp`. Он даёт -**38 agent-native инструментов**: точечное редактирование по блокам (patch / insert / delete +**39 agent-native инструментов**: точечное редактирование по блокам (patch / insert / delete по id), find/replace с сохранением структуры, скриптовые трансформации `(doc) => doc` с предпросмотром диффа, структурное редактирование таблиц, история версий с диффом / восстановлением, комментарии, изображения и ссылки на шаринг — всё применяется через слой @@ -60,7 +60,7 @@ real-time-коллаборации Docmost, поэтому запись нико | | **`/mcp` в Gitmost (наш docmost-mcp)** | Родной MCP у Docmost | | --- | :---: | :---: | | **Enterprise-лицензия** | Не нужна | Нужна | -| **Инструменты** | 38, agent-native | Примитивные (Markdown, CRUD страниц, замена целиком) | +| **Инструменты** | 39, agent-native | Примитивные (Markdown, CRUD страниц, замена целиком) | | **Правки по блокам / find-replace / скриптовые трансформации** | ✅ | — | | **Структурное редактирование таблиц, дифф / восстановление версий** | ✅ | — | | **Комментарии, изображения, ссылки на шаринг** | ✅ | — | diff --git a/packages/editor-ext/src/lib/footnote/footnote-canonicalize.ts b/packages/editor-ext/src/lib/footnote/footnote-canonicalize.ts index db543519..3d52ea5f 100644 --- a/packages/editor-ext/src/lib/footnote/footnote-canonicalize.ts +++ b/packages/editor-ext/src/lib/footnote/footnote-canonicalize.ts @@ -119,11 +119,9 @@ export function canonicalizeFootnotes(doc: T): T { } } - // 5) No references -> there must be NO list at all. + // 5) No references -> there must be NO list at all (at any depth). if (referenceIds.length === 0) { - out.content = out.content.filter( - (n: any) => !(n && n.type === FOOTNOTES_LIST_NAME), - ); + stripFootnotesListsDeep(out); return out; } @@ -147,13 +145,15 @@ export function canonicalizeFootnotes(doc: T): T { return out; } - // 7) Otherwise rebuild: strip every footnotesList and re-insert exactly one - // after the last meaningful (non-empty paragraph) block, so it coexists with - // a trailing-node empty paragraph. This both repairs a non-canonical doc and - // (in the import case) physically reorders the list into reference order. - const top: any[] = out.content.filter( - (n: any) => !(n && n.type === FOOTNOTES_LIST_NAME), - ); + // 7) Otherwise rebuild: strip every footnotesList at ANY depth (collectDefinitions + // gathers defs recursively, so a list nested in a callout/blockquote would + // otherwise have its defs copied into the new list while the original + // survives — duplicates) and re-insert exactly one after the last meaningful + // (non-empty paragraph) top-level block, so it coexists with a trailing-node + // empty paragraph. This both repairs a non-canonical doc and (in the import + // case) physically reorders the list into reference order. + stripFootnotesListsDeep(out); + const top: any[] = out.content; let insertAt = top.length; while (insertAt > 0 && isEmptyParagraph(top[insertAt - 1])) insertAt--; top.splice(insertAt, 0, { type: FOOTNOTES_LIST_NAME, content: orderedDefs }); @@ -161,10 +161,20 @@ export function canonicalizeFootnotes(doc: T): T { return out; } +/** Remove every `footnotesList` node at ANY depth (mutates the given clone). */ +function stripFootnotesListsDeep(node: any): void { + if (!node || typeof node !== 'object' || !Array.isArray(node.content)) return; + node.content = node.content.filter( + (c: any) => !(c && c.type === FOOTNOTES_LIST_NAME), + ); + for (const child of node.content) stripFootnotesListsDeep(child); +} + /** - * Order-insensitive deep equality over plain JSON (objects/arrays/primitives). - * Used to detect an already-canonical footnotesList so its physical position is - * preserved (placement parity with the live plugin). + * Deep equality over plain JSON: arrays are compared POSITIONALLY + * (order-SENSITIVE), object keys order-insensitively. The array order-sensitivity + * is required for correctness here — a reordered `footnotesList.content` must + * compare UNEQUAL so the canonical rebuild fires instead of leaving it in place. */ function deepEqualJson(a: any, b: any): boolean { if (a === b) return true; diff --git a/packages/editor-ext/tsconfig.json b/packages/editor-ext/tsconfig.json index a4ad0d72..5fcc2435 100644 --- a/packages/editor-ext/tsconfig.json +++ b/packages/editor-ext/tsconfig.json @@ -22,5 +22,11 @@ "noFallthroughCasesInSwitch": false }, "include": ["src/**/*"], - "exclude": ["node_modules", "dist", "src/**/*.spec.ts", "src/**/*.test.ts"] + "exclude": [ + "node_modules", + "dist", + "src/**/*.spec.ts", + "src/**/*.test.ts", + "src/lib/footnote/footnote-corpus.ts" + ] } diff --git a/packages/mcp/build/client.js b/packages/mcp/build/client.js index f9cf5a75..6eba7ea1 100644 --- a/packages/mcp/build/client.js +++ b/packages/mcp/build/client.js @@ -1107,9 +1107,12 @@ export class DocmostClient { } const collabToken = await this.getCollabTokenWithReauth(); let result = null; - const mutation = await mutatePageContent(pageId, collabToken, this.apiUrl, (liveDoc) => { + const mutation = await this.mutatePage(pageId, collabToken, this.apiUrl, (liveDoc) => { const r = insertInlineFootnote(liveDoc, { anchorText, text }); if (!r.inserted) { + // Abort the page-locked write by throwing: mutatePageContent does not + // persist when the transform throws, so a missing anchor leaves the + // page untouched (no partial write). throw new Error(`insert_footnote: anchor text not found: ${JSON.stringify(anchorText.slice(0, 80))}`); } result = { footnoteId: r.footnoteId, reused: r.reused }; @@ -1127,6 +1130,15 @@ export class DocmostClient { verify: mutation.verify, }; } + /** + * Page-locked write seam over collaboration.mutatePageContent. Production just + * delegates; it exists as an overridable method so the insert_footnote wrapper + * (transform abort-on-not-found + response shaping) can be unit-tested without + * standing up a live Hocuspocus collab socket. + */ + mutatePage(pageId, collabToken, apiUrl, transform) { + return mutatePageContent(pageId, collabToken, apiUrl, transform); + } /** * Export a page to a single self-contained Docmost-flavoured markdown file: * meta block + body (with inline comment anchors + diagrams) + comment diff --git a/packages/mcp/build/lib/footnote-canonicalize.js b/packages/mcp/build/lib/footnote-canonicalize.js index 92511ae1..b6673082 100644 --- a/packages/mcp/build/lib/footnote-canonicalize.js +++ b/packages/mcp/build/lib/footnote-canonicalize.js @@ -70,9 +70,10 @@ function emptyDefinition(id) { }; } /** - * Order-insensitive deep equality over plain JSON (objects/arrays/primitives). - * Used to detect an already-canonical footnotesList so its physical position is - * preserved (placement parity with the live plugin). + * Deep equality over plain JSON: arrays are compared POSITIONALLY + * (order-SENSITIVE), object keys order-insensitively. The array order-sensitivity + * is required for correctness here — a reordered `footnotesList.content` must + * compare UNEQUAL so the canonical rebuild fires instead of leaving it in place. */ function deepEqualJson(a, b) { if (a === b) @@ -148,9 +149,9 @@ export function canonicalizeFootnotes(doc) { orderedDefs.push(emptyDefinition(id)); } } - // 5) No references -> there must be NO list at all. + // 5) No references -> there must be NO list at all (at any depth). if (referenceIds.length === 0) { - out.content = out.content.filter((n) => !(n && n.type === FOOTNOTES_LIST_NAME)); + stripFootnotesListsDeep(out); return out; } // 6) Placement parity with the live plugin: when the document is ALREADY in the @@ -164,9 +165,13 @@ export function canonicalizeFootnotes(doc) { deepEqualJson(topLevelLists[0].content, orderedDefs)) { return out; } - // 7) Otherwise rebuild: strip every footnotesList and re-insert exactly one - // after the last meaningful (non-empty paragraph) block. - const top = out.content.filter((n) => !(n && n.type === FOOTNOTES_LIST_NAME)); + // 7) Otherwise rebuild: strip every footnotesList at ANY depth (collectDefinitions + // gathers defs recursively, so a list nested in a callout/blockquote would + // otherwise have its defs copied into the new list while the original + // survives — duplicates) and re-insert exactly one after the last meaningful + // (non-empty paragraph) top-level block. + stripFootnotesListsDeep(out); + const top = out.content; let insertAt = top.length; while (insertAt > 0 && isEmptyParagraph(top[insertAt - 1])) insertAt--; @@ -174,3 +179,11 @@ export function canonicalizeFootnotes(doc) { out.content = top; return out; } +/** Remove every `footnotesList` node at ANY depth (mutates the given clone). */ +function stripFootnotesListsDeep(node) { + if (!node || typeof node !== "object" || !Array.isArray(node.content)) + return; + node.content = node.content.filter((c) => !(c && c.type === FOOTNOTES_LIST_NAME)); + for (const child of node.content) + stripFootnotesListsDeep(child); +} diff --git a/packages/mcp/build/lib/transforms.js b/packages/mcp/build/lib/transforms.js index ff5862a6..9c5ecb7e 100644 --- a/packages/mcp/build/lib/transforms.js +++ b/packages/mcp/build/lib/transforms.js @@ -67,6 +67,15 @@ export function getList(doc, predicate) { }); return found; } +/** + * Textblocks that hold raw text but do NOT accept inline atom nodes. A + * `footnoteReference` is `group:"inline", atom:true`; `codeBlock` is + * `content:"text*"` (text only), so splicing a footnoteReference into it yields + * an invalid document. (paragraph/heading/detailsSummary are `inline*` and DO + * accept it; footnote definitions live inside a footnotesList which the + * footnote inserter excludes via `beforeBlock`.) + */ +const INLINE_ATOM_FORBIDDEN_BLOCKS = new Set(["codeBlock"]); /** * Insert `marker` as a PLAIN (unmarked) text run right after the first * occurrence of `anchor`. @@ -131,6 +140,14 @@ function insertNodesAfterAnchor(doc, anchor, makeMiddle, opts = {}) { // Detect whether this array is an inline array (contains text nodes). const hasText = inline.some((n) => isObject(n) && n.type === "text"); if (hasText) { + // Refuse a textblock whose content spec cannot hold the inserted nodes + // (e.g. a codeBlock for an inline atom). Keep `offset` aligned for any + // sibling textblocks in this same block, then bail so the search falls + // through to the next candidate block. + if (opts.forbidBlockTypes && opts.forbidBlockTypes.has(container.type)) { + offset += blockPlainText(container).length; + return; + } for (let i = 0; i < inline.length; i++) { const n = inline[i]; const len = isObject(n) ? blockPlainText(n).length : 0; @@ -511,7 +528,9 @@ export function commentsToFootnotes(doc, comments, opts = {}) { */ export function insertInlineFootnote(doc, opts) { const inline = mdToInlineNodes(opts.text ?? ""); - const key = footnoteContentKey(makeFootnoteDefinition("", inline)); + // footnoteContentKey only reads `.content`, so key off the inline array + // directly instead of building a throwaway definition node. + const key = footnoteContentKey({ content: inline }); // Content dedup: reuse an existing definition's id when its key matches. let footnoteId = null; let reused = false; @@ -532,8 +551,19 @@ export function insertInlineFootnote(doc, opts) { if (footnoteId == null) footnoteId = generateFootnoteId(); // Insert the footnoteReference node directly after the anchor (mark-safe - // split); it hugs the preceding word with no leading space. - const r = insertNodesAfterAnchor(doc, (opts.anchorText ?? "").trimEnd(), () => [{ type: "footnoteReference", attrs: { id: footnoteId } }]); + // split); it hugs the preceding word with no leading space. The search is + // bounded to the BODY (before the first footnotesList) and refuses codeBlocks, + // so the inline atom can never be spliced into a footnote definition or a code + // block — which would persist a schema-invalid doc (insert_footnote skips + // validateDocStructure). When the only match is in such a place the insert is + // refused and the write aborts cleanly (inserted:false). + const listIdx = Array.isArray(doc?.content) + ? doc.content.findIndex((n) => isObject(n) && n.type === "footnotesList") + : -1; + const r = insertNodesAfterAnchor(doc, (opts.anchorText ?? "").trimEnd(), () => [{ type: "footnoteReference", attrs: { id: footnoteId } }], { + ...(listIdx >= 0 ? { beforeBlock: listIdx } : {}), + forbidBlockTypes: INLINE_ATOM_FORBIDDEN_BLOCKS, + }); if (!r.inserted) { return { doc: clone(doc), inserted: false, footnoteId, reused }; } diff --git a/packages/mcp/src/client.ts b/packages/mcp/src/client.ts index 9169237d..2b449924 100644 --- a/packages/mcp/src/client.ts +++ b/packages/mcp/src/client.ts @@ -1399,13 +1399,16 @@ export class DocmostClient { } const collabToken = await this.getCollabTokenWithReauth(); let result: { footnoteId: string; reused: boolean } | null = null; - const mutation = await mutatePageContent( + const mutation = await this.mutatePage( pageId, collabToken, this.apiUrl, (liveDoc: any) => { const r = insertInlineFootnote(liveDoc, { anchorText, text }); if (!r.inserted) { + // Abort the page-locked write by throwing: mutatePageContent does not + // persist when the transform throws, so a missing anchor leaves the + // page untouched (no partial write). throw new Error( `insert_footnote: anchor text not found: ${JSON.stringify( anchorText.slice(0, 80), @@ -1429,6 +1432,21 @@ export class DocmostClient { }; } + /** + * Page-locked write seam over collaboration.mutatePageContent. Production just + * delegates; it exists as an overridable method so the insert_footnote wrapper + * (transform abort-on-not-found + response shaping) can be unit-tested without + * standing up a live Hocuspocus collab socket. + */ + protected mutatePage( + pageId: string, + collabToken: string, + apiUrl: string, + transform: (doc: any) => any, + ): Promise<{ doc?: any; verify?: any }> { + return mutatePageContent(pageId, collabToken, apiUrl, transform); + } + /** * Export a page to a single self-contained Docmost-flavoured markdown file: * meta block + body (with inline comment anchors + diagrams) + comment diff --git a/packages/mcp/src/lib/footnote-canonicalize.ts b/packages/mcp/src/lib/footnote-canonicalize.ts index d5a4a257..1e544a92 100644 --- a/packages/mcp/src/lib/footnote-canonicalize.ts +++ b/packages/mcp/src/lib/footnote-canonicalize.ts @@ -73,9 +73,10 @@ function emptyDefinition(id: string): any { } /** - * Order-insensitive deep equality over plain JSON (objects/arrays/primitives). - * Used to detect an already-canonical footnotesList so its physical position is - * preserved (placement parity with the live plugin). + * Deep equality over plain JSON: arrays are compared POSITIONALLY + * (order-SENSITIVE), object keys order-insensitively. The array order-sensitivity + * is required for correctness here — a reordered `footnotesList.content` must + * compare UNEQUAL so the canonical rebuild fires instead of leaving it in place. */ function deepEqualJson(a: any, b: any): boolean { if (a === b) return true; @@ -151,11 +152,9 @@ export function canonicalizeFootnotes(doc: T): T { } } - // 5) No references -> there must be NO list at all. + // 5) No references -> there must be NO list at all (at any depth). if (referenceIds.length === 0) { - out.content = out.content.filter( - (n: any) => !(n && n.type === FOOTNOTES_LIST_NAME), - ); + stripFootnotesListsDeep(out); return out; } @@ -175,14 +174,25 @@ export function canonicalizeFootnotes(doc: T): T { return out; } - // 7) Otherwise rebuild: strip every footnotesList and re-insert exactly one - // after the last meaningful (non-empty paragraph) block. - const top: any[] = out.content.filter( - (n: any) => !(n && n.type === FOOTNOTES_LIST_NAME), - ); + // 7) Otherwise rebuild: strip every footnotesList at ANY depth (collectDefinitions + // gathers defs recursively, so a list nested in a callout/blockquote would + // otherwise have its defs copied into the new list while the original + // survives — duplicates) and re-insert exactly one after the last meaningful + // (non-empty paragraph) top-level block. + stripFootnotesListsDeep(out); + const top: any[] = out.content; let insertAt = top.length; while (insertAt > 0 && isEmptyParagraph(top[insertAt - 1])) insertAt--; top.splice(insertAt, 0, { type: FOOTNOTES_LIST_NAME, content: orderedDefs }); out.content = top; return out; } + +/** Remove every `footnotesList` node at ANY depth (mutates the given clone). */ +function stripFootnotesListsDeep(node: any): void { + if (!node || typeof node !== "object" || !Array.isArray(node.content)) return; + node.content = node.content.filter( + (c: any) => !(c && c.type === FOOTNOTES_LIST_NAME), + ); + for (const child of node.content) stripFootnotesListsDeep(child); +} diff --git a/packages/mcp/src/lib/transforms.ts b/packages/mcp/src/lib/transforms.ts index 65313d49..639f6d9e 100644 --- a/packages/mcp/src/lib/transforms.ts +++ b/packages/mcp/src/lib/transforms.ts @@ -81,15 +81,33 @@ export function getList( return found; } -/** Options for insertMarkerAfter. */ +/** Options for insertMarkerAfter / insertNodesAfterAnchor. */ export interface InsertMarkerOptions { /** * Limit the search to TOP-LEVEL blocks with index < beforeBlock. Used to keep * footnote markers in the body and out of the notes section. */ beforeBlock?: number; + /** + * Textblock node types that MUST NOT receive the inserted nodes. When the + * split point lands inside such a block it is refused (skipped), so an inline + * ATOM (e.g. footnoteReference) is never spliced into a block whose content + * spec forbids it — which would persist a schema-invalid doc. Plain-text + * markers leave this unset (text is valid inside a codeBlock). + */ + forbidBlockTypes?: ReadonlySet; } +/** + * Textblocks that hold raw text but do NOT accept inline atom nodes. A + * `footnoteReference` is `group:"inline", atom:true`; `codeBlock` is + * `content:"text*"` (text only), so splicing a footnoteReference into it yields + * an invalid document. (paragraph/heading/detailsSummary are `inline*` and DO + * accept it; footnote definitions live inside a footnotesList which the + * footnote inserter excludes via `beforeBlock`.) + */ +const INLINE_ATOM_FORBIDDEN_BLOCKS: ReadonlySet = new Set(["codeBlock"]); + /** * Insert `marker` as a PLAIN (unmarked) text run right after the first * occurrence of `anchor`. @@ -175,6 +193,14 @@ function insertNodesAfterAnchor( (n: any) => isObject(n) && n.type === "text", ); if (hasText) { + // Refuse a textblock whose content spec cannot hold the inserted nodes + // (e.g. a codeBlock for an inline atom). Keep `offset` aligned for any + // sibling textblocks in this same block, then bail so the search falls + // through to the next candidate block. + if (opts.forbidBlockTypes && opts.forbidBlockTypes.has(container.type)) { + offset += blockPlainText(container).length; + return; + } for (let i = 0; i < inline.length; i++) { const n = inline[i]; const len = isObject(n) ? blockPlainText(n).length : 0; @@ -638,7 +664,9 @@ export function insertInlineFootnote( opts: InsertInlineFootnoteOptions, ): InsertInlineFootnoteResult { const inline = mdToInlineNodes(opts.text ?? ""); - const key = footnoteContentKey(makeFootnoteDefinition("", inline)); + // footnoteContentKey only reads `.content`, so key off the inline array + // directly instead of building a throwaway definition node. + const key = footnoteContentKey({ content: inline }); // Content dedup: reuse an existing definition's id when its key matches. let footnoteId: string | null = null; @@ -662,11 +690,25 @@ export function insertInlineFootnote( if (footnoteId == null) footnoteId = generateFootnoteId(); // Insert the footnoteReference node directly after the anchor (mark-safe - // split); it hugs the preceding word with no leading space. + // split); it hugs the preceding word with no leading space. The search is + // bounded to the BODY (before the first footnotesList) and refuses codeBlocks, + // so the inline atom can never be spliced into a footnote definition or a code + // block — which would persist a schema-invalid doc (insert_footnote skips + // validateDocStructure). When the only match is in such a place the insert is + // refused and the write aborts cleanly (inserted:false). + const listIdx = Array.isArray(doc?.content) + ? doc.content.findIndex( + (n: any) => isObject(n) && n.type === "footnotesList", + ) + : -1; const r = insertNodesAfterAnchor( doc, (opts.anchorText ?? "").trimEnd(), () => [{ type: "footnoteReference", attrs: { id: footnoteId } }], + { + ...(listIdx >= 0 ? { beforeBlock: listIdx } : {}), + forbidBlockTypes: INLINE_ATOM_FORBIDDEN_BLOCKS, + }, ); if (!r.inserted) { return { doc: clone(doc), inserted: false, footnoteId, reused }; diff --git a/packages/mcp/test/mock/footnote-write.test.mjs b/packages/mcp/test/mock/footnote-write.test.mjs index d013d7a3..29196b39 100644 --- a/packages/mcp/test/mock/footnote-write.test.mjs +++ b/packages/mcp/test/mock/footnote-write.test.mjs @@ -10,10 +10,11 @@ // These stand a local http.createServer in for Docmost and only exercise plain // HTTP routes (login / comments / pages.info), deliberately avoiding the live // Hocuspocus collab WebSocket: the insertFootnote guards short-circuit before it, -// and docmost_transform's dryRun preview never opens it. The full collab mutate -// path (abort-via-throw on a missing anchor, the reused/message response branch) -// is covered at the pure level by insertInlineFootnote in -// test/unit/footnote-canonicalize.test.mjs. +// and docmost_transform's dryRun preview never opens it. The collab mutate path +// itself — abort-via-throw on a missing anchor with NO persisted write, and the +// reused-vs-new response shaping — is covered in +// test/mock/insert-footnote-wrapper.test.mjs (which overrides the mutatePage +// seam to drive the transform), not here. import { test, after } from "node:test"; import assert from "node:assert/strict"; import http from "node:http"; diff --git a/packages/mcp/test/mock/insert-footnote-wrapper.test.mjs b/packages/mcp/test/mock/insert-footnote-wrapper.test.mjs new file mode 100644 index 00000000..887806b7 --- /dev/null +++ b/packages/mcp/test/mock/insert-footnote-wrapper.test.mjs @@ -0,0 +1,100 @@ +// Wrapper tests for DocmostClient.insertFootnote (issue #228, review #11/#9): +// the page-locked write seam (mutatePage) is overridden so the wrapper's +// transform + response shaping can be exercised WITHOUT a live Hocuspocus collab +// socket. We assert the two guarantees that the pure insertInlineFootnote test +// can NOT prove on its own: +// - a missing anchor makes the transform throw "anchor text not found" and NO +// document is persisted (the no-partial-write guarantee), and +// - a success shapes footnoteId / reused / message / verify and writes a doc +// carrying the new reference + the derived single list. +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { DocmostClient } from "../../build/client.js"; + +const para = (...c) => ({ type: "paragraph", content: c }); +const ref = (id) => ({ type: "footnoteReference", attrs: { id } }); +const def = (id, text) => ({ + type: "footnoteDefinition", + attrs: { id }, + content: [{ type: "paragraph", content: [{ type: "text", text }] }], +}); +const list = (...d) => ({ type: "footnotesList", content: d }); + +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; +} + +// A DocmostClient whose auth + page-locked write are stubbed; `mutatePage` +// mirrors collaboration.mutatePageContent (run the transform against a clone of +// the live doc; if it throws, persist NOTHING and rethrow). +function makeClient(liveDoc) { + const calls = { writes: [] }; + class TestClient extends DocmostClient { + async ensureAuthenticated() {} + async getCollabTokenWithReauth() { + return "collab-token"; + } + async mutatePage(pageId, token, apiUrl, transform) { + calls.pageId = pageId; + calls.token = token; + const newDoc = transform(structuredClone(liveDoc)); + calls.writes.push(newDoc); + return { doc: newDoc, verify: { ok: true, marker: "v" } }; + } + } + const client = new TestClient("http://127.0.0.1:1/api", "e@x.com", "pw"); + return { client, calls }; +} + +test("insertFootnote: anchor not found -> throws and persists nothing", async () => { + const { client, calls } = makeClient({ + type: "doc", + content: [para({ type: "text", text: "nothing to anchor on" })], + }); + await assert.rejects( + () => client.insertFootnote("p1", "ZZZ", "a note"), + /anchor text not found/i, + ); + assert.equal(calls.writes.length, 0, "no document may be persisted on a missing anchor"); +}); + +test("insertFootnote: success (new) writes a reference + derived list and shapes the response", async () => { + const { client, calls } = makeClient({ + type: "doc", + content: [para({ type: "text", text: "The sky is blue today." })], + }); + const res = await client.insertFootnote("p1", "blue", "Rayleigh scattering."); + assert.equal(res.success, true); + assert.equal(res.modified, true); + assert.equal(res.pageId, "p1"); + assert.equal(res.reused, false); + assert.equal(typeof res.footnoteId, "string"); + assert.ok(res.footnoteId.length > 0); + assert.equal(res.message, "Footnote inserted."); + assert.deepEqual(res.verify, { ok: true, marker: "v" }); + assert.equal(calls.writes.length, 1, "exactly one write persisted"); + assert.equal(findAll(calls.writes[0], "footnoteReference").length, 1); + assert.equal(findAll(calls.writes[0], "footnotesList").length, 1); + assert.equal(calls.pageId, "p1"); +}); + +test("insertFootnote: success (reused) reuses the existing definition and reports it", async () => { + const liveDoc = { + type: "doc", + content: [ + para({ type: "text", text: "Alpha and beta." }, ref("a")), + list(def("a", "shared note")), + ], + }; + const { client, calls } = makeClient(liveDoc); + const res = await client.insertFootnote("p1", "beta", "shared note"); + assert.equal(res.reused, true); + assert.equal(res.footnoteId, "a"); + assert.match(res.message, /reused an existing same-content definition/i); + // Still exactly one definition (the reused one), two references to it. + assert.equal(findAll(calls.writes[0], "footnoteDefinition").length, 1); + assert.equal(findAll(calls.writes[0], "footnoteReference").length, 2); +}); diff --git a/packages/mcp/test/unit/footnote-canonicalize.test.mjs b/packages/mcp/test/unit/footnote-canonicalize.test.mjs index d25a265b..c4b68ce5 100644 --- a/packages/mcp/test/unit/footnote-canonicalize.test.mjs +++ b/packages/mcp/test/unit/footnote-canonicalize.test.mjs @@ -28,52 +28,10 @@ const def = (id, text) => ({ const para = (...inline) => ({ type: "paragraph", content: inline }); const list = (...defs) => ({ type: "footnotesList", content: defs }); -test("canonicalize orders definitions by first reference (out-of-order -> 1..N)", () => { - const doc = { - type: "doc", - content: [ - para({ type: "text", text: "x" }, ref("b"), ref("a"), ref("d"), ref("c")), - list(def("a", "A"), def("c", "C"), def("b", "B"), def("d", "D")), - ], - }; - const out = canonicalizeFootnotes(doc); - assert.deepEqual(defIds(out), ["b", "a", "d", "c"]); - assert.equal(findAll(out, "footnotesList").length, 1); -}); - -test("canonicalize drops orphan definitions", () => { - const doc = { - type: "doc", - content: [ - para({ type: "text", text: "x" }, ref("a")), - list(def("a", "A"), def("orphan", "O")), - ], - }; - assert.deepEqual(defIds(canonicalizeFootnotes(doc)), ["a"]); -}); - -test("canonicalize: no references -> no list", () => { - const doc = { - type: "doc", - content: [para({ type: "text", text: "x" }), list(def("o", "O"))], - }; - const out = canonicalizeFootnotes(doc); - assert.equal(findAll(out, "footnotesList").length, 0); -}); - -test("canonicalize: duplicate definitions -> first wins, rest dropped", () => { - const doc = { - type: "doc", - content: [ - para({ type: "text", text: "x" }, ref("d")), - list(def("d", "first"), def("d", "second")), - ], - }; - const out = canonicalizeFootnotes(doc); - assert.deepEqual(defIds(out), ["d"]); - assert.match(JSON.stringify(out), /"first"/); - assert.doesNotMatch(JSON.stringify(out), /"second"/); -}); +// The ordering / orphan-drop / no-refs / duplicate-first-wins cases are covered +// (with full deepEqual on input -> expected) by the shared golden corpus in +// footnote-corpus.test.mjs; only the input-immutability and idempotence +// properties — which the corpus does not assert — are kept here. test("canonicalize is idempotent", () => { const doc = { @@ -181,6 +139,57 @@ test("insertInlineFootnote: anchor not found -> inserted:false, no write", () => assert.equal(findAll(r.doc, "footnoteReference").length, 0); }); +test("insertInlineFootnote: anchor ONLY inside a codeBlock -> refused (no invalid doc)", () => { + // A footnoteReference is an inline atom; codeBlock content is text-only, so + // splicing one in would persist a schema-invalid doc. The insert must refuse. + const doc = { + type: "doc", + content: [{ type: "codeBlock", content: [{ type: "text", text: "const blue = 1;" }] }], + }; + const r = insertInlineFootnote(doc, { anchorText: "blue", text: "Rayleigh." }); + assert.equal(r.inserted, false); + assert.equal(findAll(r.doc, "footnoteReference").length, 0); + assert.equal(findAll(r.doc, "footnotesList").length, 0); + // The codeBlock text is untouched. + assert.deepEqual(r.doc, doc); +}); + +test("insertInlineFootnote: anchor ONLY inside an existing footnote definition -> refused", () => { + // The anchor text lives in a definition (inside the footnotesList). The search + // is bounded to the BODY (before the first list), so it is not matched there + // and the insert is refused rather than nesting a reference in a definition. + const doc = { + type: "doc", + content: [ + para({ type: "text", text: "Hello world." }, ref("a")), + list(def("a", "the sky is blue")), + ], + }; + const r = insertInlineFootnote(doc, { anchorText: "sky", text: "note" }); + assert.equal(r.inserted, false); + // No EXTRA reference and still exactly one (the pre-existing) list/definition. + assert.equal(findAll(r.doc, "footnoteReference").length, 1); + assert.deepEqual(defIds(r.doc), ["a"]); +}); + +test("insertInlineFootnote: codeBlock match is skipped, a later body paragraph still anchors", () => { + // The anchor first appears in a codeBlock (refused) but also in a normal + // paragraph after it; the insert falls through to the valid block. + const doc = { + type: "doc", + content: [ + { type: "codeBlock", content: [{ type: "text", text: "let token = 1;" }] }, + para({ type: "text", text: "The token is rotated daily." }), + ], + }; + const r = insertInlineFootnote(doc, { anchorText: "token", text: "secret" }); + assert.equal(r.inserted, true); + // The reference landed in the paragraph, NOT the codeBlock. + const code = findAll(r.doc, "codeBlock")[0]; + assert.equal(findAll(code, "footnoteReference").length, 0); + assert.equal(findAll(r.doc, "footnoteReference").length, 1); +}); + test("markdown import: out-of-order definitions render as a reference-ordered list", async () => { // References appear b, a, c in the body; definitions are written in a, b, c // order (the import order). After canonicalization the bottom list follows diff --git a/packages/mcp/test/unit/footnote-corpus-parity.test.mjs b/packages/mcp/test/unit/footnote-corpus-parity.test.mjs new file mode 100644 index 00000000..5a944395 --- /dev/null +++ b/packages/mcp/test/unit/footnote-corpus-parity.test.mjs @@ -0,0 +1,49 @@ +// CI guard for architecture item B: the shared golden corpus is duplicated (the +// canonical TS copy in editor-ext + the MCP .mjs mirror), so a typo in one copy +// would otherwise pass BOTH per-package suites green while silently breaking the +// cross-copy invariant. This test loads BOTH copies and asserts they are +// deep-equal, turning "the two corpora stay identical" into a checked property. +// +// The editor-ext copy is a .ts module (not importable from node:test), so it is +// read as text and its array literal — which is pure JSON produced by +// JSON.stringify — is parsed out directly. +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 { FOOTNOTE_CORPUS as MCP_CORPUS } from "./footnote-corpus.mjs"; + +function loadEditorExtCorpus() { + const here = dirname(fileURLToPath(import.meta.url)); + const tsPath = resolve( + here, + "../../../editor-ext/src/lib/footnote/footnote-corpus.ts", + ); + const src = readFileSync(tsPath, "utf8"); + // The value is `export const FOOTNOTE_CORPUS: FootnoteCorpusCase[] = [ ... ];` + // where `[ ... ]` is strict JSON (JSON.stringify output). Slice from the + // assignment's opening bracket to the final closing bracket and parse. + const assignAt = src.indexOf("] = "); + assert.ok(assignAt >= 0, "could not locate the editor-ext corpus assignment"); + const jsonStart = src.indexOf("[", assignAt + 3); + const jsonEnd = src.lastIndexOf("]"); + assert.ok(jsonStart >= 0 && jsonEnd > jsonStart, "could not bound the corpus array"); + return JSON.parse(src.slice(jsonStart, jsonEnd + 1)); +} + +test("the editor-ext and MCP golden corpora are byte-for-byte identical", () => { + const editorExt = loadEditorExtCorpus(); + assert.ok(Array.isArray(editorExt) && editorExt.length > 0, "editor-ext corpus is non-empty"); + assert.equal( + MCP_CORPUS.length, + editorExt.length, + "the two corpora must have the same number of cases", + ); + assert.deepEqual( + MCP_CORPUS, + editorExt, + "the MCP corpus mirror has drifted from the editor-ext canonical copy — re-sync them", + ); +});