diff --git a/CHANGELOG.md b/CHANGELOG.md index dfbdb63a..d9ad3d09 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -99,6 +99,22 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `updatePageContent`). The total MCP tool count is unchanged (−1 / +1). The external names shown here are the post-#412 camelCase names. (#411) +- **`getNode` now returns Markdown by default (was ProseMirror JSON).** The + block-level read/write tools default to Markdown so a block round trip is + `getNode` (markdown) → edit → `patchNode` (markdown). `getNode` now returns + `{ …, format: "markdown", markdown }` unless you pass `format: "json"` (which + restores the previous `{ …, node }` ProseMirror subtree); comment anchors — + including resolved ones — are preserved in the markdown so a write-back never + orphans a thread, and a node that cannot be a document top-level block + (`tableRow`/`tableCell`/`tableHeader` addressed via `#`) auto-falls back + to JSON with `format: "json"` in the response. `patchNode`/`insertNode` gain a + `markdown` input alongside `node` (provide exactly one): the markdown fragment + may rewrite/insert several blocks at once and supports `^[...]` footnotes. + *Migration (external MCP clients only):* a client that consumed `getNode`'s + `node` field must now either read `markdown`, or pass `format: "json"` to keep + the old ProseMirror-JSON output. Released together with the `#411`/`#412` + breaking window so external configs break exactly once. (#413) + ### Added - **Place several images side by side in a row.** A new "Inline (side by diff --git a/apps/server/src/core/ai-chat/tools/ai-chat-tools.service.spec.ts b/apps/server/src/core/ai-chat/tools/ai-chat-tools.service.spec.ts index 00990666..78b3611d 100644 --- a/apps/server/src/core/ai-chat/tools/ai-chat-tools.service.spec.ts +++ b/apps/server/src/core/ai-chat/tools/ai-chat-tools.service.spec.ts @@ -355,23 +355,32 @@ describe('AiChatToolsService node-arg JSON-string coercion', () => { content: [{ type: 'text', text: 'Hello' }], }; - it('patchNode parses a JSON-string node and forwards it as an object', async () => { + it('patchNode parses a JSON-string node and forwards it as { node } (object)', async () => { const tools = await buildTools(); await tools.patchNode.execute( { pageId: 'p1', nodeId: 'n1', node: JSON.stringify(NODE_OBJ) } as never, {} as never, ); expect(patchNodeCalls).toHaveLength(1); - expect(patchNodeCalls[0]).toEqual(['p1', 'n1', NODE_OBJ]); + // #413: the 3rd arg is now the XOR input { markdown?, node? }. + expect(patchNodeCalls[0]).toEqual([ + 'p1', + 'n1', + { markdown: undefined, node: NODE_OBJ }, + ]); }); - it('patchNode passes an object node through unchanged', async () => { + it('patchNode passes an object node through unchanged inside { node }', async () => { const tools = await buildTools(); await tools.patchNode.execute( { pageId: 'p1', nodeId: 'n1', node: NODE_OBJ } as never, {} as never, ); - expect(patchNodeCalls[0]).toEqual(['p1', 'n1', NODE_OBJ]); + expect(patchNodeCalls[0]).toEqual([ + 'p1', + 'n1', + { markdown: undefined, node: NODE_OBJ }, + ]); }); it('patchNode throws the documented message on invalid JSON string', async () => { @@ -385,7 +394,7 @@ describe('AiChatToolsService node-arg JSON-string coercion', () => { expect(patchNodeCalls).toHaveLength(0); }); - it('insertNode parses a JSON-string node and forwards it as an object', async () => { + it('insertNode parses a JSON-string node and forwards it inside { node }', async () => { const tools = await buildTools(); await tools.insertNode.execute( { @@ -396,9 +405,15 @@ describe('AiChatToolsService node-arg JSON-string coercion', () => { {} as never, ); expect(insertNodeCalls).toHaveLength(1); - const [pageId, node] = insertNodeCalls[0]; + // #413: the 2nd arg is the XOR input { markdown?, node? }, the 3rd is opts. + const [pageId, input, opts] = insertNodeCalls[0] as [ + string, + { markdown?: unknown; node?: unknown }, + { position?: string }, + ]; expect(pageId).toBe('p1'); - expect(node).toEqual(NODE_OBJ); + expect(input).toEqual({ markdown: undefined, node: NODE_OBJ }); + expect(opts.position).toBe('append'); }); it('insertNode throws the documented message on invalid JSON string', async () => { diff --git a/apps/server/src/core/ai-chat/tools/ai-chat-tools.service.ts b/apps/server/src/core/ai-chat/tools/ai-chat-tools.service.ts index e7e0414b..d9fa8aa9 100644 --- a/apps/server/src/core/ai-chat/tools/ai-chat-tools.service.ts +++ b/apps/server/src/core/ai-chat/tools/ai-chat-tools.service.ts @@ -62,7 +62,7 @@ function __assertClientCallContract(client: DocmostClientLike): void { void client.listSidebarPages(s, s); void client.getOutline(s); void client.getPageJson(s); - void client.getNode(s, s); + void client.getNode(s, s, 'markdown'); void client.searchInPage(s, s, { regex: true, caseSensitive: true, @@ -84,12 +84,16 @@ function __assertClientCallContract(client: DocmostClientLike): void { void client.movePage(s, s, s); void client.deletePage(s); void client.editPageText(s, edits); - void client.patchNode(s, s, node); - void client.insertNode(s, node, { - position: 'append', - anchorNodeId: s, - anchorText: s, - }); + void client.patchNode(s, s, { markdown: s, node }); + void client.insertNode( + s, + { markdown: s, node }, + { + position: 'append', + anchorNodeId: s, + anchorText: s, + }, + ); void client.deleteNode(s, s); void client.updatePageJson(s, node, s); void client.tableInsertRow(s, s, cells, n); diff --git a/packages/mcp/src/client.ts b/packages/mcp/src/client.ts index 8852378f..e1ac7fa2 100644 --- a/packages/mcp/src/client.ts +++ b/packages/mcp/src/client.ts @@ -32,9 +32,12 @@ import { } from "./lib/markdown-document.js"; import { replaceNodeById, + replaceNodeByIdWithMany, + reassignCollidingBlockIds, deleteNodeById, assertUnambiguousMatch, insertNodeRelative, + insertNodesRelative, blockPlainText, buildOutline, getNodeByRef, @@ -44,6 +47,11 @@ import { updateTableCell, findInvalidNode, } from "@docmost/prosemirror-markdown"; +import { + importMarkdownFragment, + canBeDocChild, + findUnrepresentableTableAttrs, +} from "./lib/markdown-fragment.js"; import { searchInDoc, SearchOptions } from "./lib/page-search.js"; import { withPageLock } from "./lib/page-lock.js"; import { @@ -83,6 +91,7 @@ import { commentsToFootnotes, canonicalizeFootnotes, insertInlineFootnote, + mergeFootnoteDefinitions, } from "./lib/transforms.js"; import { normalizeAndMergeFootnotes } from "./lib/footnote-normalize-merge.js"; import vm from "node:vm"; @@ -1298,12 +1307,31 @@ export class DocmostClient { } /** - * Fetch a single node's full ProseMirror subtree (lossless) by reference: - * a block id (headings/paragraphs/callouts/images), or `#` to select - * a top-level block by its outline index (the only way to reach tables/rows/ - * cells, which carry no id). + * Fetch a single block for editing by reference: a block id (headings/ + * paragraphs/callouts/images), or `#` to select a top-level block by its + * outline index (the only way to reach tables/rows/cells, which carry no id). + * + * `format` (#413): + * - `"markdown"` (DEFAULT): serialize the block via the canonical converter + * (`{type:"doc",content:[node]}` -> `convertProseMirrorToMarkdown`) — a read + * "for editing": pair it with `patchNode({markdown})` to rewrite the block. + * Comment anchors (``, INCLUDING resolved ones) are + * NOT stripped here (unlike getPage): losing them on write-back would + * orphan the thread. Returns `{ ..., format:"markdown", markdown }`. + * - `"json"`: return the raw ProseMirror subtree as-is (lossless; the previous + * default). Returns `{ ..., format:"json", node }`. + * + * AUTO fallback: a type that cannot be a document top-level child + * (tableRow/tableCell/tableHeader, addressed by `#`) is NOT expressible + * as a standalone markdown document, so a `"markdown"` request for such a node + * transparently falls back to JSON with an explicit `format:"json"` field. The + * check derives from the schema's `doc` contentMatch, so it tracks the schema. */ - async getNode(pageId: string, nodeId: string) { + async getNode( + pageId: string, + nodeId: string, + format: "markdown" | "json" = "markdown", + ) { await this.ensureAuthenticated(); const data = await this.getPageRaw(pageId); const hit = getNodeByRef( @@ -1315,12 +1343,35 @@ export class DocmostClient { `getNode: no node found for "${nodeId}" on page ${pageId} (use a block id from getOutline, or "#" for a top-level block such as a table)`, ); } + + // JSON requested (or a non-top-level type that markdown cannot represent as a + // standalone document): return the subtree verbatim. + if (format === "json" || !canBeDocChild(hit.type)) { + return { + pageId, + ref: nodeId, + path: hit.path, + type: hit.type, + format: "json" as const, + node: hit.node, + }; + } + + // Markdown: wrap the node as a one-block doc and run the canonical converter. + // Comment anchors are DELIBERATELY preserved (converter default) so a + // getNode(markdown) -> edit -> patchNode(markdown) round trip does not orphan + // a comment thread; this differs from getPage, which strips them. + const markdown = convertProseMirrorToMarkdown({ + type: "doc", + content: [hit.node], + }); return { pageId, ref: nodeId, path: hit.path, type: hit.type, - node: hit.node, + format: "markdown" as const, + markdown, }; } @@ -2307,17 +2358,61 @@ export class DocmostClient { } /** - * Replace EVERY node whose attrs.id === nodeId (recursively, including nodes - * nested in callouts/tables) with the supplied node. Operates on the LIVE - * collab document so comments and concurrent edits are preserved. + * Replace the block whose attrs.id === nodeId. Operates on the LIVE collab + * document so comments and concurrent edits are preserved. * - * The replacement node's block id is preserved: if node.attrs is missing it - * is created, and if node.attrs.id is missing it is set to nodeId so the - * replacement keeps the same id it replaced. Throws if no node matches. + * Exactly one of `input.markdown` / `input.node` (#413): + * - `markdown` (RECOMMENDED): the block is rewritten from a canonical markdown + * fragment. The fragment may import to N blocks (a "1 -> N" splice: rewrite a + * whole section in one call). The FIRST resulting block INHERITS the target's + * `attrs.id` (so an existing comment anchoring the block by id survives); the + * rest get FRESH ids. `^[...]` footnotes in the fragment are first-class: + * their definitions merge into the page's TAIL footnote list (content-key + * dedup + canonicalize), same machinery insertFootnote uses. REJECTED when + * the TARGET block carries a table-cell attribute markdown cannot represent + * (colspan/rowspan/colwidth/background) — use the table tools or `node`. + * - `node`: a raw ProseMirror node for precise attr/mark work. The replacement + * keeps the target id (if `node.attrs.id` is missing it is set to nodeId). + * + * #159 ambiguous-id semantics are unchanged: 0 matches -> "no node"; >1 matches + * -> "ambiguous, refused" (nothing written), on BOTH paths — the markdown path + * runs a dry `replaceNodeById` count first, so a duplicated id never splices. */ - async patchNode(pageId: string, nodeId: string, node: any) { + async patchNode( + pageId: string, + nodeId: string, + input: { markdown?: string; node?: any }, + ) { await this.ensureAuthenticated(); + // XOR: exactly one of markdown / node. Both optional in the schema; the + // runtime enforces the recommendation ("markdown for prose, node for fine + // work") without letting an ambiguous both-or-neither call through. + const hasMd = + input != null && + typeof input.markdown === "string" && + input.markdown.trim() !== ""; + const hasNode = input != null && input.node != null; + if (hasMd === hasNode) { + throw new Error( + "patchNode: provide exactly one of `markdown` (recommended, for prose) " + + "or `node` (a raw ProseMirror node, for precise attr/mark work)", + ); + } + + if (hasMd) { + return this.patchNodeMarkdown(pageId, nodeId, input.markdown as string); + } + return this.patchNodeJson(pageId, nodeId, input.node); + } + + /** + * patchNode with a raw ProseMirror `node` (the pre-#413 behavior). Replaces + * EVERY node whose attrs.id === nodeId; the swapped-in node keeps the target + * id. #159 ambiguity refused. Split out so the markdown path can reuse the + * shared collab/guard plumbing without a giant branch. + */ + private async patchNodeJson(pageId: string, nodeId: string, node: any) { if (!node || typeof node !== "object" || typeof node.type !== "string") { throw new Error( "patchNode: `node` must be an object with a string `type`", @@ -2382,22 +2477,143 @@ export class DocmostClient { } /** - * Insert a node relative to an anchor (or append it at the top level). + * patchNode with a MARKDOWN fragment (#413). Imports the fragment through the + * canonical importer, then 1 -> N splices the resulting blocks in place of the + * target block on the LIVE collab doc: + * - the FIRST block inherits the target's id; the rest get FRESH ids (minted + * by the importer/id-remap, so neighbour blocks are untouched); + * - `^[...]` footnote definitions merge into the page's tail list; + * - REJECTED when the target block carries a markdown-unrepresentable table + * attr (colspan/rowspan/colwidth/background) — guarding against silent loss; + * - #159 ambiguity is enforced by a dry `replaceNodeById` count BEFORE the + * splice, so a duplicated id never writes. + */ + private async patchNodeMarkdown( + pageId: string, + nodeId: string, + markdown: string, + ) { + // Import the fragment up front (network-free, canonical) so a bad fragment + // fails before any collab connection or page lock. + const { blocks, definitions } = await importMarkdownFragment(markdown); + + // The first imported block inherits the target id; the rest keep the fresh + // ids the importer assigned. Build the thread now so it is stable across a + // collab retry (the transform below is pure over its inputs). + const threaded = blocks.map((b, i) => { + if (i !== 0) return b; + return { + ...b, + attrs: { + ...(b && typeof b.attrs === "object" ? b.attrs : {}), + id: nodeId, + }, + }; + }); + + // Shape-validate every imported block up front (parity with the JSON path): + // the importer only emits schema nodes, but the check is cheap insurance and + // yields the same rich #409 diagnostics if the schema ever drifts. + for (const b of threaded) { + this.assertValidNodeShape("patchNode", b); + } + + const collabToken = await this.getCollabTokenWithReauth(); + // Open the collab doc by the canonical UUID, never the slugId (#260). + const pageUuid = await this.resolvePageId(pageId); + + let replaced = 0; + let guardAttrs: string | null = null; + const mutation = await mutatePageContent( + pageUuid, + collabToken, + this.apiUrl, + (liveDoc) => { + replaced = 0; + guardAttrs = null; + + // #159: count matches with the same recursive walk the JSON path uses; + // only an UNAMBIGUOUS single match may write. A dry count keeps the + // ambiguity semantics identical across both paths. + const { replaced: count } = replaceNodeById(liveDoc, nodeId, { + type: "paragraph", + }); + replaced = count; + if (count !== 1) return null; + + // Guard against SILENT LOSS: if the target block carries a table-cell + // attribute markdown cannot represent (colspan/rowspan/colwidth/ + // background), refuse the markdown rewrite so those attrs are not + // dropped. Simple tables (no such attrs) rewrite fine. + const hit = getNodeByRef(liveDoc, nodeId); + guardAttrs = hit ? findUnrepresentableTableAttrs(hit.node) : null; + if (guardAttrs != null) return null; + + // Re-mint any minted block id that collides with an existing page id + // (skip index 0: its id is intentionally the target nodeId, unique by + // the #159 dry-count above), so the 1 -> N splice stays page-wide unique. + reassignCollidingBlockIds(liveDoc, threaded, 0); + + // 1 -> N splice, then merge any fragment footnote definitions into the + // page's tail list and re-derive canonical footnote numbering. + const { doc: spliced } = replaceNodeByIdWithMany( + liveDoc, + nodeId, + threaded, + ); + return mergeFootnoteDefinitions(spliced, definitions); + }, + ); + + // Surface the guard rejection with an actionable message (nothing written). + if (guardAttrs != null) { + throw new Error( + `patchNode: the target block has table-cell attributes markdown cannot ` + + `represent (${guardAttrs}) — a markdown rewrite would drop them. Use ` + + `the table tools (tableUpdateCell/tableInsertRow) or pass a raw ` + + `ProseMirror \`node\` instead of \`markdown\`.`, + ); + } + + // 0 -> "no node"; >1 -> "ambiguous, refused" (the transform skipped the write + // for any count !== 1). Shared #159 guard, identical to the JSON path. + assertUnambiguousMatch("patchNode", "replace", replaced, nodeId, pageId); + + return { + success: true, + replaced, + nodeId, + blocks: threaded.length, + verify: mutation.verify, + }; + } + + /** + * Insert content relative to an anchor (or append it at the top level). * Operates on the LIVE collab document so comments and concurrent edits are * preserved. * + * Exactly one of `input.markdown` / `input.node` (#413): + * - `markdown` (RECOMMENDED): a canonical markdown fragment. It may import to + * SEVERAL blocks — they are inserted IN ORDER at the anchor. `^[...]` + * footnote definitions merge into the page's tail list (same machinery as + * insertFootnote). Every inserted block gets a fresh id. + * - `node`: a raw ProseMirror node for precise attr/mark work, or to insert + * table structure (a bare tableRow/tableCell/tableHeader — NOT expressible in + * markdown, so those stay JSON-only). + * * opts.position: - * - "append": push the node at the end of the top-level content. - * - "before"/"after": insert the node as a sibling of the anchor, just - * before/after it. Exactly one of anchorNodeId / anchorText must be given; - * anchorNodeId locates a node anywhere by attrs.id, anchorText matches the - * first top-level block whose plain text includes it. + * - "append": push the content at the end of the top-level content. + * - "before"/"after": insert as a sibling of the anchor, just before/after it. + * Exactly one of anchorNodeId / anchorText must be given; anchorNodeId + * locates a node anywhere by attrs.id, anchorText matches the first top-level + * block whose plain text includes it. * * Throws if the anchor cannot be found. */ async insertNode( pageId: string, - node: any, + input: { markdown?: string; node?: any }, opts: { position: "before" | "after" | "append"; anchorNodeId?: string; @@ -2406,11 +2622,19 @@ export class DocmostClient { ) { await this.ensureAuthenticated(); - if (!node || typeof node !== "object" || typeof node.type !== "string") { + // XOR: exactly one of markdown / node (both optional in the schema). + const hasMd = + input != null && + typeof input.markdown === "string" && + input.markdown.trim() !== ""; + const hasNode = input != null && input.node != null; + if (hasMd === hasNode) { throw new Error( - "insertNode: `node` must be an object with a string `type`", + "insertNode: provide exactly one of `markdown` (recommended, for prose) " + + "or `node` (a raw ProseMirror node, for precise attr/mark work or table structure)", ); } + if ( !opts || (opts.position !== "before" && @@ -2434,10 +2658,32 @@ export class DocmostClient { } } + // Resolve the ordered list of blocks to insert plus any footnote definitions + // to merge. The markdown path imports canonically (so an inserted block is + // byte-identical to the same content in a full-page import); the node path is + // a single block with no footnote merge (raw JSON `^[...]` is not touched). + let blocks: any[]; + let definitions: any[] = []; + if (hasMd) { + const frag = await importMarkdownFragment(input.markdown as string); + blocks = frag.blocks; + definitions = frag.definitions; + } else { + const node = input.node; + if (!node || typeof node !== "object" || typeof node.type !== "string") { + throw new Error( + "insertNode: `node` must be an object with a string `type`", + ); + } + blocks = [node]; + } + // #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("insertNode", node); + for (const b of blocks) { + this.assertValidNodeShape("insertNode", b); + } const collabToken = await this.getCollabTokenWithReauth(); // Open the collab doc by the canonical UUID, never the slugId (#260). @@ -2452,14 +2698,20 @@ export class DocmostClient { this.apiUrl, (liveDoc) => { inserted = false; - const { doc: nd, inserted: ins } = insertNodeRelative( - liveDoc, - node, - opts, - ); - inserted = ins; + // Re-mint any minted block id that collides with an existing page id + // (all inserted blocks are fresh, no skip) so the splice stays unique. + if (hasMd) reassignCollidingBlockIds(liveDoc, blocks); + // Single-block node path keeps `insertNodeRelative` (it owns the + // structural table-node splicing); the markdown path uses the array + // splice so N blocks land in order at one anchor. + const res = hasMd + ? insertNodesRelative(liveDoc, blocks, opts) + : insertNodeRelative(liveDoc, blocks[0], opts); + inserted = res.inserted; if (!inserted) return null; // anchor not found -> skip the write entirely - return nd; + // Merge any fragment footnote definitions into the page tail list and + // re-derive canonical numbering (no-op when there are none). + return mergeFootnoteDefinitions(res.doc, definitions); }, ); @@ -2482,6 +2734,7 @@ export class DocmostClient { success: true, inserted: true, position: opts.position, + blocks: blocks.length, verify: mutation.verify, }; } diff --git a/packages/mcp/src/lib/markdown-fragment.ts b/packages/mcp/src/lib/markdown-fragment.ts new file mode 100644 index 00000000..79796212 --- /dev/null +++ b/packages/mcp/src/lib/markdown-fragment.ts @@ -0,0 +1,243 @@ +/** + * Single-BLOCK markdown fragment support for `patch_node` / `insert_node` + * (#413). These tools accept EITHER a raw ProseMirror `node` (fine attr/mark + * work) OR a `markdown` string (the recommended default): a small markdown + * fragment is run through the canonical importer, yielding the SAME topology a + * full-page markdown import would — so a block written via markdown is + * canonically identical to the same content imported whole (no "second canon"). + * + * The importer produces a full `{type:"doc", content:[...blocks..., footnotesList?]}`. + * A fragment write needs the BLOCKS separately from the footnote DEFINITIONS so + * the caller can splice the blocks into the live document and merge the + * definitions into the page's TAIL footnote list via the existing footnote + * machinery (`insertInlineFootnote`'s `appendDefinition` + `canonicalizeFootnotes`). + * + * Footnote id-collision safety: the importer assigns sequential ids (`fn-1`, + * `fn-2`, …) starting from 1 for EVERY fragment, so a fragment's `fn-1` would + * collide with an existing page footnote also numbered `fn-1` — and + * `canonicalizeFootnotes` matches references to definitions BY id, so the + * fragment's reference would silently re-hang onto the page's unrelated + * definition. To make the merge safe regardless of the page's current numbering, + * every fragment footnote id is REMAPPED to a fresh uuid (via the importer's own + * `generateFootnoteId`) across BOTH the references (inside the blocks) and the + * definitions before either is handed back. Content-identical notes still merge + * downstream via `normalizeAndMergeFootnotes` (content-key), and the whole doc is + * renumbered by `canonicalizeFootnotes`, so the caller-visible numbering stays + * canonical. + */ + +import { markdownToProseMirror } from "./collaboration.js"; +import { generateFootnoteId } from "@docmost/prosemirror-markdown"; +import { docmostSchema } from "./docmost-schema.js"; + +/** True if `value` is a non-null, non-array object. */ +function isObject(value: any): value is Record { + return value != null && typeof value === "object" && !Array.isArray(value); +} + +/** + * Deep-walk `node` collecting every footnote id it uses (on `footnoteReference` + * and `footnoteDefinition` nodes) and build a stable OLD->NEW remap, minting a + * fresh uuid per distinct old id. The map is shared across a fragment's blocks + * and definitions so a reference and its definition receive the SAME new id. + */ +function buildFootnoteIdRemap(nodes: any[]): Map { + const remap = new Map(); + const visit = (node: any): void => { + if (!isObject(node)) return; + if ( + (node.type === "footnoteReference" || + node.type === "footnoteDefinition") && + isObject(node.attrs) && + typeof node.attrs.id === "string" && + node.attrs.id !== "" + ) { + if (!remap.has(node.attrs.id)) { + remap.set(node.attrs.id, generateFootnoteId()); + } + } + if (Array.isArray(node.content)) { + for (const child of node.content) visit(child); + } + }; + for (const n of nodes) visit(n); + return remap; +} + +/** Rewrite every footnote id in `node` IN PLACE using `remap` (deep). */ +function applyFootnoteIdRemap(node: any, remap: Map): void { + if (!isObject(node)) return; + if ( + (node.type === "footnoteReference" || node.type === "footnoteDefinition") && + isObject(node.attrs) && + typeof node.attrs.id === "string" + ) { + const next = remap.get(node.attrs.id); + if (next) node.attrs.id = next; + } + if (Array.isArray(node.content)) { + for (const child of node.content) applyFootnoteIdRemap(child, remap); + } +} + +/** + * Generate a short random block id for an imported block that arrives without one + * (the markdown importer emits `attrs.id: null`). Mirrors the mcp `freshId` + * convention (base36 random, unique within one document). The patch path then + * OVERWRITES the first block's id with the target id; every other block keeps the + * fresh id minted here — so a 1 -> N section rewrite yields addressable, + * comment-anchorable blocks rather than a run of null-id paragraphs. + */ +function freshBlockId(): string { + return ( + Math.random().toString(36).slice(2, 12) + + Math.random().toString(36).slice(2, 6) + ); +} + +/** + * Assign a fresh id to every top-level block whose `attrs.id` is null/missing, + * IN PLACE. Only the block's own id is touched (not descendants — those keep the + * importer's structure). Ensures each imported block is independently addressable. + */ +function assignFreshBlockIds(blocks: any[]): void { + for (const b of blocks) { + if (!isObject(b)) continue; + if (!isObject(b.attrs)) b.attrs = {}; + if (b.attrs.id == null || b.attrs.id === "") { + b.attrs.id = freshBlockId(); + } + } +} + +/** The parsed shape of a markdown fragment: its blocks + footnote definitions. */ +export interface MarkdownFragment { + /** Top-level blocks, in order, with the trailing `footnotesList` removed. */ + blocks: any[]; + /** + * The `footnoteDefinition` nodes lifted from the imported `footnotesList`, with + * ids already remapped to match the references left inside `blocks`. Empty when + * the fragment used no footnotes. + */ + definitions: any[]; +} + +/** + * Import a markdown fragment and return its blocks separately from its footnote + * definitions, with all footnote ids remapped to fresh uuids (see the file + * header). The importer's `^[body]` inline-footnote handling is used verbatim — + * `^[...]` in the fragment is a first-class footnote, NOT rejected — so the + * markdown path matches the full-page import exactly. + * + * Throws when the fragment imports to zero blocks (an empty / whitespace-only + * markdown string is not a valid block write). + */ +export async function importMarkdownFragment( + markdown: string, +): Promise { + const doc = await markdownToProseMirror(markdown); + const content: any[] = Array.isArray(doc?.content) ? doc.content : []; + + const blocks: any[] = []; + const definitions: any[] = []; + for (const node of content) { + if (isObject(node) && node.type === "footnotesList") { + // Lift the definitions out of the list; the list wrapper itself is + // reconstructed on the page by the canonicalizer after the merge. + if (Array.isArray(node.content)) { + for (const def of node.content) { + if (isObject(def) && def.type === "footnoteDefinition") { + definitions.push(def); + } + } + } + continue; + } + blocks.push(node); + } + + if (blocks.length === 0) { + throw new Error( + "markdown fragment produced no blocks — provide non-empty markdown, or use `node` for a raw ProseMirror node", + ); + } + + // Remap footnote ids across BOTH blocks and definitions so a fragment `fn-1` + // cannot collide with a page footnote of the same number. + const remap = buildFootnoteIdRemap([...blocks, ...definitions]); + if (remap.size > 0) { + for (const b of blocks) applyFootnoteIdRemap(b, remap); + for (const d of definitions) applyFootnoteIdRemap(d, remap); + } + + // Every top-level block needs a stable id (the importer leaves them null). The + // patch path OVERWRITES the first block's id with the target id afterwards. + assignFreshBlockIds(blocks); + + return { blocks, definitions }; +} + +/** + * True when `type` is a valid TOP-LEVEL child of the document node per the + * canonical schema's content model — i.e. `get_node` can serialize it to + * markdown by wrapping it in `{type:"doc",content:[node]}`. Derived from the + * schema's `doc` contentMatch (NOT a hand-written type list) so it tracks the + * schema automatically: `tableRow`/`tableCell`/`tableHeader` (addressed only via + * `#`) are NOT doc children and yield false, so `get_node` auto-falls back + * to JSON for them. + */ +export function canBeDocChild(type: string | undefined): boolean { + if (typeof type !== "string") return false; + const nodeType = docmostSchema.nodes[type]; + if (!nodeType) return false; + return docmostSchema.nodes.doc.contentMatch.matchType(nodeType) != null; +} + +/** + * Table-cell attributes that CANNOT survive a markdown round-trip: the converter + * emits colspan/rowspan (and align) as HTML `` cell attrs, but silently + * drops `colwidth`, `backgroundColor`, and `backgroundColorName`. A markdown + * `patch_node` on a block that carries any of these (a merged / colored / + * fixed-width cell) would therefore lose them — so it is REJECTED, pointing the + * caller at the table tools or the raw-`node` JSON path. `align` is intentionally + * absent: it round-trips as GFM alignment. + */ +function cellCarriesUnrepresentableAttrs(node: any): boolean { + if (!isObject(node)) return false; + if (node.type !== "tableCell" && node.type !== "tableHeader") return false; + const a = isObject(node.attrs) ? node.attrs : {}; + if ((a.colspan ?? 1) > 1) return true; + if ((a.rowspan ?? 1) > 1) return true; + if (a.colwidth != null) return true; + if (a.backgroundColor != null) return true; + if (a.backgroundColorName != null) return true; + return false; +} + +/** + * Scan a target block (the node being replaced) for any table cell carrying an + * attribute markdown cannot represent (colspan/rowspan/colwidth/background). When + * one is found, return a human-readable list of the offending attr NAMES so the + * caller can build an actionable rejection message; return null when the block is + * safe to rewrite from markdown. Deep — a colored cell nested inside a table + * inside a callout is still caught. + */ +export function findUnrepresentableTableAttrs(node: any): string | null { + const found = new Set(); + const visit = (n: any): void => { + if (!isObject(n)) return; + if (cellCarriesUnrepresentableAttrs(n)) { + const a = isObject(n.attrs) ? n.attrs : {}; + if ((a.colspan ?? 1) > 1) found.add("colspan"); + if ((a.rowspan ?? 1) > 1) found.add("rowspan"); + if (a.colwidth != null) found.add("colwidth"); + if (a.backgroundColor != null) found.add("backgroundColor"); + if (a.backgroundColorName != null) found.add("backgroundColorName"); + } + if (Array.isArray(n.content)) { + for (const child of n.content) visit(child); + } + }; + visit(node); + return found.size > 0 ? Array.from(found).sort().join(", ") : null; +} diff --git a/packages/mcp/src/lib/transforms.ts b/packages/mcp/src/lib/transforms.ts index c65e30e1..ac19898c 100644 --- a/packages/mcp/src/lib/transforms.ts +++ b/packages/mcp/src/lib/transforms.ts @@ -774,6 +774,61 @@ export function insertInlineFootnote( return { doc: working, inserted: true, footnoteId, reused }; } +/** + * Merge an ARRAY of footnote definitions (e.g. the definitions lifted from an + * imported markdown FRAGMENT) into `doc`\'s footnote list, then re-derive the + * canonical footnote topology — the SAME two-step machinery `insertInlineFootnote` + * uses (`appendDefinition` -> `normalizeAndMergeFootnotes` -> `canonicalizeFootnotes`). + * + * The fragment\'s `footnoteReference` nodes are assumed to ALREADY be spliced into + * `doc` (inside the just-inserted blocks) with ids matching these definitions, so + * after appending the definitions the canonicalizer orders/numbers everything by + * first-reference order, merges content-identical notes, and drops any orphan. + * Same documented caveat as every other write path: full canonicalization drops a + * definition no reference points at. + * + * NOT merely a no-op when `definitions` is empty: it still canonicalizes when + * the (post-splice) `doc` carries footnote artifacts (a `footnotesList` or any + * `footnoteReference`), so a splice that removed the LAST referrer of a page + * footnote drops the now-orphaned definition — matching a full page re-import + * (which always canonicalizes) and preserving the "canonically identical to the + * same content imported whole" invariant. A truly footnote-free doc (no artifacts + * and no definitions) is returned untouched — the fast path, no clone. When the + * work runs it goes through the pure passes (which clone), so the caller\'s `doc` + * is not mutated. + */ +export function mergeFootnoteDefinitions(doc: any, definitions: any[]): any { + const defs = Array.isArray(definitions) ? definitions : []; + // True fast path ONLY when there is nothing to merge AND nothing to canonicalize + // away; otherwise fall through so an orphan left by a splice is still dropped. + if (defs.length === 0 && !hasFootnoteArtifacts(doc)) return doc; + // Clone before appending: `appendDefinition` mutates in place, and the caller + // must not see a half-merged doc if a later pass throws. + let working = clone(doc); + for (const def of defs) { + appendDefinition(working, def); + } + // #419: normalize + merge glyph-forked definitions before canonicalizing. + working = normalizeAndMergeFootnotes(working); + working = canonicalizeFootnotes(working); + return working; +} + +/** + * True if `doc`'s tree contains any `footnotesList` node OR any + * `footnoteReference` node. Used to decide whether an empty-`definitions` merge + * must still canonicalize (to drop an orphan a splice left behind). + */ +function hasFootnoteArtifacts(doc: any): boolean { + let found = false; + walk(doc, (n) => { + if (isObject(n) && (n.type === "footnotesList" || n.type === "footnoteReference")) { + found = true; + } + }); + return found; +} + /** * Append a definition node so the canonicalizer can order/place it: into the * first existing footnotesList, or a new trailing list when none exists. diff --git a/packages/mcp/src/server-instructions.ts b/packages/mcp/src/server-instructions.ts index 03dc170b..d3a6b74b 100644 --- a/packages/mcp/src/server-instructions.ts +++ b/packages/mcp/src/server-instructions.ts @@ -40,8 +40,8 @@ import { SHARED_TOOL_SPECS, SharedToolSpec } from "./tool-specs.js"; */ export const ROUTING_PROSE = "Docmost editing guide — choose the tool by intent. The at the end lists every tool with a one-line purpose; the notes below are the routing hints for WHEN to reach for each.\n" + - "READ: find a page -> search (workspace-wide full-text); list -> listPages / listSpaces. Locate blocks and their ids CHEAPLY -> getOutline (compact top-level map; start here, not getPageJson). One block's subtree -> getNode (by attrs.id, or \"#\" for tables, which carry no id). Find every occurrence of a string/regex ON a page (and where each is) -> searchInPage, NOT block-by-block getNode — it returns each hit's node ref + block index + context for a targeted comment. Whole page -> getPage (Markdown, lossy; inline tags are comment anchors — markup, not text) or getPageJson (lossless ProseMirror with block ids). Hand a huge page (with images) to an external consumer without pulling it through the model context -> stashPage (returns a short-lived anonymous URL).\n" + - "EDIT: fix wording/typos/numbers -> editPageText (find/replace inside blocks, no node id needed). Change ONE block (paragraph/heading/callout/etc.) structurally -> patchNode (by attrs.id from getOutline). Add a block -> insertNode (before/after a block by attrs.id or by anchor text, or append). Remove a block -> deleteNode (by attrs.id). Tables -> tableGet / tableUpdateCell / tableInsertRow / tableDeleteRow (address by \"#\" from getOutline; table nodes have no attrs.id). Images -> insertImage (add from a web URL) / replaceImage (swap an existing image). Draw.io diagrams -> drawioCreate (create from mxGraph XML and insert), drawioGet (read a diagram as mxGraph XML + a hash), drawioUpdate (replace a diagram; pass the hash from drawioGet as baseHash for optimistic locking); before authoring a diagram, drawioShapes (look up verified stencil style-strings so a shape name never renders as an empty box) and drawioGuide (on-demand authoring reference: skeleton/layout/containers/icons-aws/icons-azure), and pass layout:\"elk\" to drawioCreate/drawioUpdate to auto-place nodes. Footnotes -> insertFootnote. Bulk/structural rewrite -> updatePageJson (full ProseMirror replace) or updatePageMarkdown (full plain-Markdown body replace, re-imported — block ids regenerate); prefer the granular tools above to avoid resending the whole ~100KB+ document. Complex/scripted rewrite (multiple coordinated edits, renumbering) -> docmostTransform: write a JS `(doc, ctx) => doc` transform, preview the diff with dryRun (default), then apply with dryRun:false; ctx.helpers includes commentsToFootnotes for turning inline comments into numbered footnotes.\n" + + "READ: find a page -> search (workspace-wide full-text); list -> listPages / listSpaces. Locate blocks and their ids CHEAPLY -> getOutline (compact top-level map; start here, not getPageJson). One block, for editing -> getNode (by attrs.id, or \"#\" for tables, which carry no id) — returns MARKDOWN by default (comment anchors kept for safe write-back); pass format:\"json\" for the raw ProseMirror subtree. Find every occurrence of a string/regex ON a page (and where each is) -> searchInPage, NOT block-by-block getNode — it returns each hit's node ref + block index + context for a targeted comment. Whole page -> getPage (Markdown, lossy; inline tags are comment anchors — markup, not text) or getPageJson (lossless ProseMirror with block ids). Hand a huge page (with images) to an external consumer without pulling it through the model context -> stashPage (returns a short-lived anonymous URL).\n" + + "EDIT: fix wording/typos/numbers -> editPageText (find/replace inside blocks, no node id needed). Edit a block -> getNode(markdown) -> edit the markdown -> patchNode(markdown) (by attrs.id from getOutline; the markdown fragment may be several blocks — a 1->N section rewrite in one call, the first block keeps the id). Reach for patchNode's `node`-JSON only for fine attr/mark work; a table cell with spans/colors/fixed width -> the table tools (patchNode markdown refuses it). Add a block -> insertNode (markdown, before/after a block by attrs.id or by anchor text, or append; `node` for raw JSON or bare table structure). Remove a block -> deleteNode (by attrs.id). Tables -> tableGet / tableUpdateCell / tableInsertRow / tableDeleteRow (address by \"#\" from getOutline; table nodes have no attrs.id). Images -> insertImage (add from a web URL) / replaceImage (swap an existing image). Draw.io diagrams -> drawioCreate (create from mxGraph XML and insert), drawioGet (read a diagram as mxGraph XML + a hash), drawioUpdate (replace a diagram; pass the hash from drawioGet as baseHash for optimistic locking); before authoring a diagram, drawioShapes (look up verified stencil style-strings so a shape name never renders as an empty box) and drawioGuide (on-demand authoring reference: skeleton/layout/containers/icons-aws/icons-azure), and pass layout:\"elk\" to drawioCreate/drawioUpdate to auto-place nodes. Footnotes -> insertFootnote. Bulk/structural rewrite -> updatePageJson (full ProseMirror replace) or updatePageMarkdown (full plain-Markdown body replace, re-imported — block ids regenerate); prefer the granular tools above to avoid resending the whole ~100KB+ document. Complex/scripted rewrite (multiple coordinated edits, renumbering) -> docmostTransform: write a JS `(doc, ctx) => doc` transform, preview the diff with dryRun (default), then apply with dryRun:false; ctx.helpers includes commentsToFootnotes for turning inline comments into numbered footnotes.\n" + "PAGES: new -> createPage (Markdown). Rename (title only) -> renamePage. Move -> movePage. Delete -> deletePage (SOFT delete — the page goes to trash and is restorable; nothing is permanent). Copy/replace a page's whole content from another page (server-side, no document through the model) -> copyPageContent. Sharing -> sharePage / unsharePage / listShares; sharePage makes the page PUBLICLY accessible — do it only when explicitly asked.\n" + "COMMENTS: createComment is always inline and requires an EXACT selection — contiguous text from a single block, <=250 chars (fails rather than leaving an unanchored comment); reply to a thread via parentCommentId. Propose a concrete text fix for one-click human approval -> createComment with suggestedText (the exact plain-text replacement for the selection; the selection must then be UNIQUE in the page — extend it with context if needed); prefer this over editing directly when the change is subjective or needs the author's sign-off. Manage -> listComments, updateComment, resolveComment (resolve/reopen, reversible — prefer over delete to close), deleteComment, checkNewComments.\n" + "HISTORY: review what changed -> diffPageVersions (a historyId vs current, or two versions). List saved versions -> listPageHistory. Undo a bad edit -> restorePageVersion (writes a past version back as current; itself revertible). Export a page to self-contained Docmost Markdown (with comment anchors) -> exportPageMarkdown."; diff --git a/packages/mcp/src/tool-specs.ts b/packages/mcp/src/tool-specs.ts index 874b4619..e2488f7c 100644 --- a/packages/mcp/src/tool-specs.ts +++ b/packages/mcp/src/tool-specs.ts @@ -305,22 +305,41 @@ export const SHARED_TOOL_SPECS = { mcpName: 'getNode', inAppKey: 'getNode', description: - "Fetch a single node's full ProseMirror subtree (lossless) without " + - 'pulling the whole document. `nodeId` is a block id from the page ' + + "Fetch a single block for editing. `nodeId` is a block id from the page " + 'outline or page-JSON view (works for headings/paragraphs/callouts/images), OR ' + '`#` to fetch a top-level block by its outline index — use the ' + - '`#` form for tables/rows/cells, which carry no id.', + '`#` form for tables/rows/cells, which carry no id. ' + + "`format` defaults to \"markdown\": the block is returned as a canonical " + + 'markdown fragment (comment anchors are KEPT so a patchNode write-back does ' + + 'not orphan a thread) — edit it and write it back with patchNode({markdown}). ' + + 'Pass format:"json" for the raw lossless ProseMirror subtree (for precise ' + + 'attr/mark work). A node that cannot be a document top-level block ' + + '(tableRow/tableCell/tableHeader via "#") auto-falls back to JSON with ' + + 'format:"json" in the response.', tier: 'core', catalogLine: - "getNode — fetch one block's ProseMirror subtree by block id or #index.", + "getNode — fetch one block (markdown by default; json for the raw subtree).", buildShape: (z) => ({ pageId: z.string().min(1), nodeId: z.string().min(1), + format: z + .enum(['markdown', 'json']) + .optional() + .describe( + 'Output format: "markdown" (default, for editing → patchNode) or ' + + '"json" (raw ProseMirror subtree). A non-top-level type auto-falls ' + + 'back to json.', + ), }), - execute: (client, { pageId, nodeId }) => - client.getNode(pageId as string, nodeId as string), + execute: (client, { pageId, nodeId, format }) => + client.getNode( + pageId as string, + nodeId as string, + format as 'markdown' | 'json' | undefined, + ), }, + // --- in-page occurrence search (client-side, over ProseMirror plain text) --- searchInPage: { @@ -415,24 +434,30 @@ export const SHARED_TOOL_SPECS = { mcpName: 'patchNode', inAppKey: 'patchNode', description: - 'Replace a single content block identified by its attrs.id with a new ' + - 'ProseMirror node, WITHOUT resending the whole document; the replacement ' + - 'keeps the same node id. Get the block id from the page outline (cheap) ' + - 'or the page-JSON view, then ' + - 'pass a ProseMirror node to put in its place. Example node: a paragraph ' + - '{"type":"paragraph","content":[{"type":"text","text":"Hello"}]} or a ' + - 'heading {"type":"heading","attrs":{"level":2},"content":' + + 'Replace a single content block identified by its attrs.id, WITHOUT ' + + 'resending the whole document; the replacement keeps the same block id. ' + + 'Get the block id from the page outline (cheap) or the page-JSON view. ' + + 'Provide EXACTLY ONE of `markdown` or `node`. ' + + '`markdown` (RECOMMENDED for prose): a canonical markdown fragment — the ' + + 'usual round trip is getNode (markdown) → edit the markdown → patchNode ' + + '(markdown). The fragment may be SEVERAL blocks (a "1 → N" splice: rewrite a ' + + 'whole section in one call) — the first block inherits this block id, the ' + + 'rest get fresh ids. `^[...]` footnotes are supported (their definitions ' + + "merge into the page's footnote list). REJECTED when the target is a table " + + 'cell with attributes markdown cannot represent (merged/colored/fixed-width) ' + + '— use the table tools or `node`. ' + + '`node` (for precise attr/mark work): a raw ProseMirror node, e.g. a ' + + 'paragraph {"type":"paragraph","content":[{"type":"text","text":"Hello"}]} ' + + 'or a heading {"type":"heading","attrs":{"level":2},"content":' + '[{"type":"text","text":"Title"}]}. Bold is a mark: ' + - '{"type":"text","text":"x","marks":[{"type":"bold"}]}. The node may be a ' + - '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: ' + + '{"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). ' + + 'The node may be a JSON object or a JSON string (both accepted). Reversible: ' + 'the previous version is kept in page history.', tier: 'deferred', catalogLine: - 'patchNode — replace one block with a new ProseMirror node, keeping its id.', + 'patchNode — rewrite one block from markdown (or a raw node), keeping its id.', buildShape: (z) => ({ pageId: z.string().min(1).describe('ID of the page containing the block'), nodeId: z @@ -442,35 +467,53 @@ export const SHARED_TOOL_SPECS = { 'attrs.id of the block to replace (from the page outline or ' + 'page-JSON view)', ), + markdown: z + .string() + .optional() + .describe( + 'RECOMMENDED. Canonical markdown to replace the block with; may be ' + + 'several blocks (the first inherits the id, the rest get fresh ids). ' + + 'Exactly one of markdown / node.', + ), node: z .any() + .optional() .describe( - 'ProseMirror node to put in place of the node with this id, e.g. ' + + 'For precise attr/mark work: a ProseMirror node to put in place of ' + + 'the block, e.g. ' + '{"type":"paragraph","content":[{"type":"text","text":"Hello"}]}. ' + - 'JSON object or JSON string both accepted.', + 'JSON object or JSON string both accepted. Exactly one of markdown / node.', ), }), // parseNodeArg normalizes a JSON-string node into an object (the model // sometimes serializes it as a string) before the client's typeof-object - // guard rejects it — identical on both hosts. - execute: (client, { pageId, nodeId, node }) => - client.patchNode(pageId as string, nodeId as string, parseNodeArg(node)), + // guard rejects it — identical on both hosts. The XOR (markdown vs node) is + // enforced at runtime in the client (both schema-optional). + execute: (client, { pageId, nodeId, markdown, node }) => + client.patchNode(pageId as string, nodeId as string, { + markdown: markdown as string | undefined, + node: node == null ? undefined : parseNodeArg(node), + }), }, + insertNode: { mcpName: 'insertNode', inAppKey: 'insertNode', description: - 'Insert a block before/after another block (by attrs.id or anchor text) ' + + 'Insert content before/after another block (by attrs.id or anchor text) ' + 'or append it at the end (top level). For before/after you MUST provide ' + 'EXACTLY ONE of anchorNodeId or anchorText. Get anchor block ids from the ' + 'page outline or the page-JSON view. Avoids resending the whole document. ' + - 'Can also insert ' + - 'table structure: to add a tableRow, pass a tableRow node with position ' + - 'before/after and anchor INSIDE the target table — anchorNodeId of any ' + - 'block/cell in it, or anchorText matching the table; to add a ' + - 'tableCell/tableHeader, use anchorNodeId of a block inside the target row ' + - '(anchorText only resolves top-level blocks, so it cannot target a row). ' + + 'Provide EXACTLY ONE of `markdown` or `node`. ' + + '`markdown` (RECOMMENDED): a canonical markdown fragment — may be SEVERAL ' + + 'blocks, inserted in order at the anchor; `^[...]` footnotes supported. ' + + '`node` (for precise attr/mark work OR table structure): a raw ProseMirror ' + + 'node. Table structure is JSON-only (not expressible in markdown): to add a ' + + 'tableRow, pass a tableRow node with position before/after and anchor INSIDE ' + + 'the target table — anchorNodeId of any block/cell in it, or anchorText ' + + 'matching the table; to add a tableCell/tableHeader, use anchorNodeId of a ' + + 'block inside the target row (anchorText only resolves top-level blocks). ' + "`anchorText` is matched against the block's literal rendered plain text " + '(no markdown); markdown/emoji are tolerated as a fallback; prefer plain ' + 'text or anchorNodeId. Note: append is top-level only and rejects ' + @@ -485,15 +528,23 @@ export const SHARED_TOOL_SPECS = { 'JSON object or a JSON string (both accepted). Reversible via page history.', tier: 'deferred', catalogLine: - 'insertNode — insert a block before/after an anchor, or append at the end.', + 'insertNode — insert markdown (or a raw node) before/after an anchor, or append.', buildShape: (z) => ({ pageId: z.string().min(1), + markdown: z + .string() + .optional() + .describe( + 'RECOMMENDED. Canonical markdown to insert; may be several blocks ' + + '(inserted in order). Exactly one of markdown / node.', + ), node: z .any() + .optional() .describe( - 'ProseMirror node to insert, e.g. ' + - '{"type":"paragraph","content":[{"type":"text","text":"Hello"}]}. ' + - 'JSON object or JSON string both accepted.', + 'For precise attr/mark work or table structure: a ProseMirror node, ' + + 'e.g. {"type":"paragraph","content":[{"type":"text","text":"Hello"}]}. ' + + 'JSON object or JSON string both accepted. Exactly one of markdown / node.', ), position: z .enum(['before', 'after', 'append']) @@ -511,14 +562,27 @@ export const SHARED_TOOL_SPECS = { 'are tolerated as a fallback; prefer plain text or anchorNodeId.', ), }), - execute: (client, { pageId, node, position, anchorNodeId, anchorText }) => - client.insertNode(pageId as string, parseNodeArg(node), { - position: position as 'before' | 'after' | 'append', - anchorNodeId: anchorNodeId as string | undefined, - anchorText: anchorText as string | undefined, - }), + // The XOR (markdown vs node) is enforced at runtime in the client (both + // schema-optional). parseNodeArg only runs on the node path. + execute: ( + client, + { pageId, markdown, node, position, anchorNodeId, anchorText }, + ) => + client.insertNode( + pageId as string, + { + markdown: markdown as string | undefined, + node: node == null ? undefined : parseNodeArg(node), + }, + { + position: position as 'before' | 'after' | 'append', + anchorNodeId: anchorNodeId as string | undefined, + anchorText: anchorText as string | undefined, + }, + ), }, + // --- share management --- // Unified from the per-layer inline definitions (#294). Both layers already diff --git a/packages/mcp/test/mock/ambiguous-node-id.test.mjs b/packages/mcp/test/mock/ambiguous-node-id.test.mjs index 03eaf9bd..d011a40a 100644 --- a/packages/mcp/test/mock/ambiguous-node-id.test.mjs +++ b/packages/mcp/test/mock/ambiguous-node-id.test.mjs @@ -133,8 +133,10 @@ test("patchNode REFUSES an ambiguous (duplicate) id without writing to collab", await assert.rejects( () => client.patchNode("11111111-1111-4111-8111-111111111111", DUP_ID, { - type: "paragraph", - content: [{ type: "text", text: "replacement" }], + node: { + type: "paragraph", + content: [{ type: "text", text: "replacement" }], + }, }), /ambiguous/i, "patchNode must reject a duplicate-id target with an 'ambiguous' error", diff --git a/packages/mcp/test/mock/get-node-format.test.mjs b/packages/mcp/test/mock/get-node-format.test.mjs new file mode 100644 index 00000000..996e7187 --- /dev/null +++ b/packages/mcp/test/mock/get-node-format.test.mjs @@ -0,0 +1,157 @@ +// #413: getNode's markdown-default format, its JSON opt-in, the non-top-level +// AUTO fallback to JSON, and comment-anchor preservation (incl. resolved) on the +// markdown read. getNode only reads (getPageRaw), so a lightweight subclass that +// stubs auth + the page fetch is enough — no collab socket needed. +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { DocmostClient } from "../../build/client.js"; + +function makeClient(doc) { + class TestClient extends DocmostClient { + async ensureAuthenticated() {} + async getPageRaw(pageId) { + return { id: pageId, slugId: "s", title: "P", spaceId: "sp", content: doc }; + } + } + return new TestClient("http://127.0.0.1:1/api", "e@x.com", "pw"); +} + +const P = "p1"; + +test("getNode defaults to markdown for a paragraph", async () => { + const doc = { + type: "doc", + content: [ + { + type: "paragraph", + attrs: { id: "b1" }, + content: [{ type: "text", text: "hello world" }], + }, + ], + }; + const res = await makeClient(doc).getNode(P, "b1"); + assert.equal(res.format, "markdown"); + assert.equal(typeof res.markdown, "string"); + assert.match(res.markdown, /hello world/); + assert.equal(res.node, undefined, "markdown result carries no raw node"); +}); + +test("getNode format:'json' returns the raw subtree verbatim", async () => { + const target = { + type: "paragraph", + attrs: { id: "b1" }, + content: [{ type: "text", text: "hello" }], + }; + const doc = { type: "doc", content: [target] }; + const res = await makeClient(doc).getNode(P, "b1", "json"); + assert.equal(res.format, "json"); + assert.deepEqual(res.node, target); + assert.equal(res.markdown, undefined); +}); + +test("getNode AUTO-falls back to JSON for a non-top-level type (tableRow via #index)", async () => { + const doc = { + type: "doc", + content: [ + { + type: "table", + content: [ + { + type: "tableRow", + content: [ + { + type: "tableCell", + attrs: { colspan: 1, rowspan: 1 }, + content: [ + { + type: "paragraph", + attrs: { id: "cp" }, + content: [{ type: "text", text: "x" }], + }, + ], + }, + ], + }, + ], + }, + ], + }; + // "#0.0"-style refs are not supported; the whole table is "#0", a row is only + // reachable by drilling — but a tableRow IS a non-doc-child type. Address the + // table itself as "#0": a table CAN be a doc child, so markdown is fine there. + // To hit the fallback, address the row by walking: getNode resolves "#0" to the + // table (doc child -> markdown). Instead we verify the schema gate directly by + // asking for the table (markdown) and a row is exercised via the unit test on + // canBeDocChild; here confirm a table renders as markdown. + const tableRes = await makeClient(doc).getNode(P, "#0"); + assert.equal(tableRes.format, "markdown", "a table is a doc child -> markdown"); + + // Now build a doc whose top-level block IS a tableRow (schematically invalid but + // exercises the getNode fallback branch): getNode("#0") resolves it and, because + // tableRow cannot be a doc child, must fall back to JSON. + const rowDoc = { + type: "doc", + content: [ + { + type: "tableRow", + content: [ + { + type: "tableCell", + attrs: { colspan: 1, rowspan: 1 }, + content: [{ type: "paragraph", content: [{ type: "text", text: "y" }] }], + }, + ], + }, + ], + }; + const rowRes = await makeClient(rowDoc).getNode(P, "#0"); + assert.equal(rowRes.format, "json", "a tableRow cannot be a doc child -> JSON fallback"); + assert.equal(rowRes.type, "tableRow"); + assert.ok(rowRes.node, "the JSON fallback returns the raw subtree"); +}); + +test("getNode(markdown) PRESERVES comment anchors — active and resolved", async () => { + // A paragraph with two comment marks: one active, one resolved. get_page strips + // resolved anchors; getNode must NOT (a read for editing/write-back). + const doc = { + type: "doc", + content: [ + { + type: "paragraph", + attrs: { id: "b1" }, + content: [ + { type: "text", text: "start " }, + { + type: "text", + text: "active", + marks: [{ type: "comment", attrs: { commentId: "cid-active" } }], + }, + { type: "text", text: " mid " }, + { + type: "text", + text: "resolved", + marks: [ + { + type: "comment", + attrs: { commentId: "cid-resolved", resolved: true }, + }, + ], + }, + { type: "text", text: " end" }, + ], + }, + ], + }; + const res = await makeClient(doc).getNode(P, "b1"); + assert.equal(res.format, "markdown"); + assert.match( + res.markdown, + /data-comment-id="cid-active"/, + "the active comment anchor is preserved", + ); + assert.match( + res.markdown, + /data-comment-id="cid-resolved"/, + "the RESOLVED comment anchor is ALSO preserved (unlike get_page)", + ); +}); diff --git a/packages/mcp/test/mock/invalid-node-validation.test.mjs b/packages/mcp/test/mock/invalid-node-validation.test.mjs index bb534581..6331dedc 100644 --- a/packages/mcp/test/mock/invalid-node-validation.test.mjs +++ b/packages/mcp/test/mock/invalid-node-validation.test.mjs @@ -135,7 +135,7 @@ test("patchNode fails fast on a nested typeless node — no collab connection", const client = new DocmostClient(baseURL, "user@example.com", "pw"); await assert.rejects( - () => client.patchNode(PAGE, SEED_ID, nestedTypelessNode()), + () => client.patchNode(PAGE, SEED_ID, { node: nestedTypelessNode() }), (err) => { assert.match(err.message, /patchNode: invalid node/); assert.match(err.message, /missing "type"/); @@ -158,9 +158,13 @@ test("insertNode fails fast on a nested UNKNOWN type — no collab connection", await assert.rejects( () => - client.insertNode(PAGE, nestedUnknownTypeNode(), { - position: "append", - }), + client.insertNode( + PAGE, + { node: nestedUnknownTypeNode() }, + { + position: "append", + }, + ), (err) => { assert.match(err.message, /insertNode: invalid node/); assert.match(err.message, /unknown node type "paragraf"/); @@ -225,8 +229,10 @@ test("patchNode with a well-formed node proceeds to the collab write", async () const client = new DocmostClient(baseURL, "user@example.com", "pw"); const result = await client.patchNode(PAGE, SEED_ID, { - type: "paragraph", - content: [{ type: "text", text: "replacement" }], + node: { + type: "paragraph", + content: [{ type: "text", text: "replacement" }], + }, }); assert.equal(result.success, true); diff --git a/packages/mcp/test/mock/markdown-patch-insert.test.mjs b/packages/mcp/test/mock/markdown-patch-insert.test.mjs new file mode 100644 index 00000000..a6b843eb --- /dev/null +++ b/packages/mcp/test/mock/markdown-patch-insert.test.mjs @@ -0,0 +1,538 @@ +// Mock collab tests for the #413 MARKDOWN path of patchNode / insertNode and the +// markdown-default getNode. These stand up a real Hocuspocus collab server seeded +// with a chosen document (mirroring ambiguous-node-id.test.mjs), let the client +// run its real transform against a live Y.Doc, and read the persisted result back +// to assert on the written document. +// +// Coverage (issue #413): +// - CANON CONVERGENCE: a block written via patchNode(markdown) is canonically +// equal to the SAME content run through a full markdown import (no "second +// canon" appears on the block-level path). +// - id-THREAD on a 1->N splice: the first block inherits the target id, the rest +// get fresh ids, and every NEIGHBOUR block is byte-identical before/after. +// - XOR validation (both / neither markdown+node -> error). +// - span/color-attr GUARD on the target block (a merged/colored cell refuses a +// markdown patch, nothing written). +// - `^[...]` footnote in the fragment -> a definition in the tail list + renumber. +// - insertNode(markdown) inserts N blocks in order at the anchor. +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"; +import { + docsCanonicallyEqual, + markdownToProseMirror, +} from "@docmost/prosemirror-markdown"; + +const PAGE = "11111111-1111-4111-8111-111111111111"; + +// Deep JSON clone for byte-identity assertions. +const jclone = (v) => JSON.parse(JSON.stringify(v)); + +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; +} + +// Stand up an HTTP+Hocuspocus stack seeded with `seedDoc`. `state.lastDoc` holds +// the most recently persisted document JSON (decoded from the live Y.Doc on every +// change) so a test can inspect exactly what was written. +async function spawnCollabStack(seedDoc) { + const state = { changed: false, lastDoc: null }; + + const hocuspocus = new Hocuspocus({ + quiet: true, + async onLoadDocument() { + return buildYDoc(seedDoc); + }, + async onChange(data) { + state.changed = true; + try { + const frag = data.document.getXmlFragment("default"); + // Decode the live fragment back to JSON via the same helper the client + // reads with — but simpler: use the yjs->json path exposed by the doc. + state.lastDoc = fragmentToJson(frag); + } catch { + /* ignore decode errors in teardown races */ + } + }, + }); + + 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") { + 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 }; +} + +// Minimal XmlFragment -> ProseMirror JSON decode, mirroring the shape Docmost +// stores. Reads element name as node type, attributes as attrs, and recurses into +// children; text nodes carry their string. +function fragmentToJson(frag) { + const decodeNode = (el) => { + if (el.constructor.name === "YXmlText") { + // A yjs text node: collect the string with its formatting deltas. + const delta = el.toDelta(); + return delta.map((d) => { + const node = { type: "text", text: d.insert }; + if (d.attributes && Object.keys(d.attributes).length) { + node.marks = Object.entries(d.attributes).map(([type, attrs]) => + attrs && typeof attrs === "object" && Object.keys(attrs).length + ? { type, attrs } + : { type }, + ); + } + return node; + }); + } + const node = { type: el.nodeName }; + const attrs = el.getAttributes(); + if (attrs && Object.keys(attrs).length) node.attrs = attrs; + const children = []; + for (const child of el.toArray()) { + const decoded = decodeNode(child); + if (Array.isArray(decoded)) children.push(...decoded); + else children.push(decoded); + } + if (children.length) node.content = children; + return node; + }; + const content = []; + for (const child of frag.toArray()) content.push(decodeNode(child)); + return { type: "doc", content }; +} + +const openStacks = []; +after(async () => { + await Promise.all( + openStacks.map( + ({ server, hocuspocus }) => + new Promise((resolve) => { + server.close(() => { + Promise.resolve(hocuspocus.destroy?.()).finally(resolve); + }); + }), + ), + ); +}); + +// A seed doc with two neighbour paragraphs around a target paragraph. +function seed3() { + return { + type: "doc", + content: [ + { + type: "paragraph", + attrs: { id: "before-id" }, + content: [{ type: "text", text: "before" }], + }, + { + type: "paragraph", + attrs: { id: "target-id" }, + content: [{ type: "text", text: "old target" }], + }, + { + type: "paragraph", + attrs: { id: "after-id" }, + content: [{ type: "text", text: "after" }], + }, + ], + }; +} + +test("patchNode(markdown): XOR — both markdown and node is rejected, nothing written", async () => { + const { state, baseURL } = await spawnCollabStack(seed3()); + const client = new DocmostClient(baseURL, "e@x.com", "pw"); + await assert.rejects( + () => + client.patchNode(PAGE, "target-id", { + markdown: "hello", + node: { type: "paragraph" }, + }), + /exactly one of/i, + ); + assert.equal(state.changed, false, "no write on an XOR violation"); +}); + +test("patchNode(markdown): XOR — neither markdown nor node is rejected", async () => { + const { baseURL } = await spawnCollabStack(seed3()); + const client = new DocmostClient(baseURL, "e@x.com", "pw"); + await assert.rejects( + () => client.patchNode(PAGE, "target-id", {}), + /exactly one of/i, + ); +}); + +test("patchNode(markdown): single block keeps the id; neighbours byte-identical", async () => { + const before = seed3(); + const { state, baseURL } = await spawnCollabStack(before); + const client = new DocmostClient(baseURL, "e@x.com", "pw"); + + const res = await client.patchNode(PAGE, "target-id", { + markdown: "the **new** target", + }); + assert.equal(res.success, true); + assert.equal(res.replaced, 1); + assert.equal(res.blocks, 1); + + const doc = state.lastDoc; + const paras = doc.content; + // The rewritten block still carries the target id. + const target = paras.find((p) => p.attrs?.id === "target-id"); + assert.ok(target, "rewritten block inherits target-id"); + assert.equal(target.content.some((n) => n.text === "new"), true); + // Neighbours are byte-identical to the seed. + const beforeNode = paras.find((p) => p.attrs?.id === "before-id"); + const afterNode = paras.find((p) => p.attrs?.id === "after-id"); + assert.deepEqual(beforeNode, before.content[0]); + assert.deepEqual(afterNode, before.content[2]); +}); + +test("patchNode(markdown): 1->N splice threads the id onto the first block; neighbours byte-identical", async () => { + const before = seed3(); + const { state, baseURL } = await spawnCollabStack(before); + const client = new DocmostClient(baseURL, "e@x.com", "pw"); + + // Two paragraphs of markdown -> a 2-block fragment replacing one block. + const res = await client.patchNode(PAGE, "target-id", { + markdown: "first para\n\nsecond para", + }); + assert.equal(res.blocks, 2); + + const doc = state.lastDoc; + const idx = doc.content.findIndex((p) => p.attrs?.id === "target-id"); + assert.ok(idx >= 0, "first spliced block inherits target-id"); + const first = doc.content[idx]; + const second = doc.content[idx + 1]; + assert.equal(first.content.some((n) => n.text === "first para"), true); + assert.equal(second.content.some((n) => n.text === "second para"), true); + // The second block has a DIFFERENT (fresh) id. + assert.notEqual(second.attrs?.id, "target-id"); + assert.ok(second.attrs?.id, "the extra block gets a fresh id"); + // Neighbours untouched, byte-identical. + assert.deepEqual( + doc.content.find((p) => p.attrs?.id === "before-id"), + before.content[0], + ); + assert.deepEqual( + doc.content.find((p) => p.attrs?.id === "after-id"), + before.content[2], + ); +}); + +test("patchNode(markdown): CANON CONVERGENCE — block equals the same content full-imported", async () => { + const { state, baseURL } = await spawnCollabStack(seed3()); + const client = new DocmostClient(baseURL, "e@x.com", "pw"); + + const md = "a paragraph with **bold**, _italic_ and `code`"; + await client.patchNode(PAGE, "target-id", { markdown: md }); + + // The block as persisted. + const target = state.lastDoc.content.find((p) => p.attrs?.id === "target-id"); + // The same markdown run through the full-page importer. + const full = await markdownToProseMirror(md); + const fullBlock = full.content[0]; + + assert.ok( + docsCanonicallyEqual( + { type: "doc", content: [target] }, + { type: "doc", content: [fullBlock] }, + ), + "a patchNode(markdown) block must be canonically equal to a full import — no second canon", + ); +}); + +test("patchNode(markdown): a paragraph inside a merged (colspan) cell rewrites fine — the cell's span is preserved", async () => { + // A cell paragraph carries an id and IS id-targetable; rewriting ITS content + // from markdown replaces only the paragraph, so the cell's colspan is NOT lost + // (the span lives on the cell, which patchNode leaves in place). This is the + // correct behavior: no false guard, no loss. The guard's REJECTION logic (when + // the replaced block itself carries/contains an unrepresentable span) is proven + // by the findUnrepresentableTableAttrs unit test — that case is not reachable + // through the id-targeting API because tables/cells carry no addressable id. + const doc = { + type: "doc", + content: [ + { + type: "table", + content: [ + { + type: "tableRow", + content: [ + { + type: "tableCell", + attrs: { colspan: 2, rowspan: 1 }, + content: [ + { + type: "paragraph", + attrs: { id: "cell-para" }, + content: [{ type: "text", text: "merged" }], + }, + ], + }, + ], + }, + ], + }, + ], + }; + const { state, baseURL } = await spawnCollabStack(doc); + const client = new DocmostClient(baseURL, "e@x.com", "pw"); + + const res = await client.patchNode(PAGE, "cell-para", { markdown: "rewritten" }); + assert.equal(res.success, true); + // The cell's colspan survives (the span is on the cell, not the paragraph). + const cell = findAll(state.lastDoc, "tableCell")[0]; + assert.equal(cell.attrs.colspan, 2, "the cell's colspan is preserved"); + const para = findAll(cell, "paragraph")[0]; + assert.equal( + (para.content || []).some((n) => n.text === "rewritten"), + true, + ); +}); + +test("patchNode(markdown): a `^[...]` footnote in the fragment lands in the tail list", async () => { + const { state, baseURL } = await spawnCollabStack(seed3()); + const client = new DocmostClient(baseURL, "e@x.com", "pw"); + + await client.patchNode(PAGE, "target-id", { + markdown: "a claim^[the supporting note]", + }); + + const doc = state.lastDoc; + const lists = findAll(doc, "footnotesList"); + assert.equal(lists.length, 1, "exactly one tail footnotesList"); + const defs = findAll(doc, "footnoteDefinition"); + assert.equal(defs.length, 1, "one definition for the fragment footnote"); + const refs = findAll(doc, "footnoteReference"); + assert.equal(refs.length, 1, "one reference in the body"); + // Reference and definition share an id (renumbered canonically). + assert.equal(refs[0].attrs.id, defs[0].attrs.id); +}); + +test("insertNode(markdown): inserts N blocks in order after the anchor", async () => { + const before = seed3(); + const { state, baseURL } = await spawnCollabStack(before); + const client = new DocmostClient(baseURL, "e@x.com", "pw"); + + const res = await client.insertNode( + PAGE, + { markdown: "new one\n\nnew two" }, + { position: "after", anchorNodeId: "before-id" }, + ); + assert.equal(res.success, true); + assert.equal(res.blocks, 2); + + const texts = state.lastDoc.content.map((p) => (p.content || []).map((n) => n.text).join("")); + // Order: before, new one, new two, target, after. + assert.deepEqual(texts, ["before", "new one", "new two", "old target", "after"]); +}); + +test("insertNode(markdown): XOR — both markdown and node is rejected", async () => { + const { baseURL } = await spawnCollabStack(seed3()); + const client = new DocmostClient(baseURL, "e@x.com", "pw"); + await assert.rejects( + () => + client.insertNode( + PAGE, + { markdown: "x", node: { type: "paragraph" } }, + { position: "append" }, + ), + /exactly one of/i, + ); +}); + +// A seed page whose ONLY footnote reference lives in the target paragraph p1, +// with a matching definition in a trailing footnotesList. Rewriting p1 with a +// footnote-free fragment removes the last referrer -> the definition is orphaned. +function seedOrphanFootnote() { + return { + type: "doc", + content: [ + { + type: "paragraph", + attrs: { id: "p1" }, + content: [ + { type: "text", text: "a claim" }, + { type: "footnoteReference", attrs: { id: "fn-1", referenceNumber: 1 } }, + ], + }, + { + type: "footnotesList", + content: [ + { + type: "footnoteDefinition", + attrs: { id: "fn-1" }, + content: [ + { + type: "paragraph", + attrs: { id: "def-para" }, + content: [{ type: "text", text: "the supporting note" }], + }, + ], + }, + ], + }, + ], + }; +} + +test("patchNode(markdown): removing the LAST footnote referrer drops the now-orphan definition (canonical convergence)", async () => { + const { state, baseURL } = await spawnCollabStack(seedOrphanFootnote()); + const client = new DocmostClient(baseURL, "e@x.com", "pw"); + + // The fragment has NO footnotes -> definitions=[]; the splice removes the only + // footnoteReference, leaving the tail definition orphaned. The canonicalization + // pass (which mergeFootnoteDefinitions must still run) has to drop it. + await client.patchNode(PAGE, "p1", { markdown: "just text" }); + + const doc = state.lastDoc; + assert.equal( + findAll(doc, "footnoteDefinition").length, + 0, + "the orphaned definition is dropped", + ); + assert.equal( + findAll(doc, "footnotesList").length, + 0, + "the emptied footnotesList is removed", + ); + assert.equal(findAll(doc, "footnoteReference").length, 0, "no references remain"); + + // Convergence: the persisted result equals the SAME content imported whole. + const full = await markdownToProseMirror("just text"); + const target = doc.content.find((p) => p.attrs?.id === "p1"); + assert.ok( + docsCanonicallyEqual( + { type: "doc", content: [target] }, + { type: "doc", content: [full.content[0]] }, + ), + "the post-splice doc is canonically identical to a full re-import", + ); +}); + +test("patchNode(markdown): a pure-text patch on a footnote-FREE page leaves footnote topology untouched (fast path)", async () => { + const before = seed3(); + const { state, baseURL } = await spawnCollabStack(before); + const client = new DocmostClient(baseURL, "e@x.com", "pw"); + + await client.patchNode(PAGE, "target-id", { markdown: "plain replacement" }); + + const doc = state.lastDoc; + assert.equal(findAll(doc, "footnotesList").length, 0, "no footnotesList appears"); + assert.equal(findAll(doc, "footnoteDefinition").length, 0, "no definition appears"); + assert.equal(findAll(doc, "footnoteReference").length, 0, "no reference appears"); + // Neighbours byte-identical (the fast path does not clone/reshape the tree). + assert.deepEqual( + doc.content.find((p) => p.attrs?.id === "before-id"), + before.content[0], + ); + assert.deepEqual( + doc.content.find((p) => p.attrs?.id === "after-id"), + before.content[2], + ); +}); + +test("insertNode(markdown): a footnote-free insert on a page carrying a footnote still canonicalizes (definitions empty)", async () => { + // The page has an existing footnote (ref + tail def). Inserting a footnote-free + // fragment keeps the reference alive, so the definition stays — but the write + // path must still run canonicalization (definitions=[]), producing exactly one + // tail list with the reference/definition ids in sync. + const { state, baseURL } = await spawnCollabStack(seedOrphanFootnote()); + const client = new DocmostClient(baseURL, "e@x.com", "pw"); + + const res = await client.insertNode( + PAGE, + { markdown: "unrelated one\n\nunrelated two" }, + { position: "after", anchorNodeId: "p1" }, + ); + assert.equal(res.success, true); + + const doc = state.lastDoc; + assert.equal(findAll(doc, "footnoteReference").length, 1, "the existing reference survives"); + assert.equal(findAll(doc, "footnotesList").length, 1, "exactly one tail list"); + const defs = findAll(doc, "footnoteDefinition"); + assert.equal(defs.length, 1, "the definition is kept (still referenced)"); + assert.equal(findAll(doc, "footnoteReference")[0].attrs.id, defs[0].attrs.id); +}); + +// Collect every TOP-LEVEL block id in a doc (the invariant the splice dedup +// guarantees is page-wide top-level uniqueness). +function topLevelIds(doc) { + return doc.content + .map((b) => b?.attrs?.id) + .filter((id) => id != null); +} + +test("patchNode(markdown): a 1->N splice yields page-wide UNIQUE top-level block ids", async () => { + const before = seed3(); + const { state, baseURL } = await spawnCollabStack(before); + const client = new DocmostClient(baseURL, "e@x.com", "pw"); + + await client.patchNode(PAGE, "target-id", { + markdown: "one\n\ntwo\n\nthree", + }); + + const ids = topLevelIds(state.lastDoc); + assert.equal(new Set(ids).size, ids.length, "all top-level block ids are unique"); + // The target id is still present (threaded onto the first block). + assert.ok(ids.includes("target-id"), "the first block still inherits target-id"); +}); + +test("insertNode(markdown): inserting multiple blocks yields page-wide UNIQUE top-level block ids", async () => { + const before = seed3(); + const { state, baseURL } = await spawnCollabStack(before); + const client = new DocmostClient(baseURL, "e@x.com", "pw"); + + await client.insertNode( + PAGE, + { markdown: "alpha\n\nbeta\n\ngamma" }, + { position: "after", anchorNodeId: "before-id" }, + ); + + const ids = topLevelIds(state.lastDoc); + assert.equal(new Set(ids).size, ids.length, "all top-level block ids are unique"); +}); diff --git a/packages/mcp/test/unit/markdown-fragment.test.mjs b/packages/mcp/test/unit/markdown-fragment.test.mjs new file mode 100644 index 00000000..e704a8a5 --- /dev/null +++ b/packages/mcp/test/unit/markdown-fragment.test.mjs @@ -0,0 +1,121 @@ +// #413: unit tests for the markdown-fragment helpers used by patchNode/insertNode. +import { test } from "node:test"; +import assert from "node:assert/strict"; + +import { + importMarkdownFragment, + canBeDocChild, + findUnrepresentableTableAttrs, +} from "../../build/lib/markdown-fragment.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; +} + +test("importMarkdownFragment: plain markdown -> blocks, no definitions", async () => { + const { blocks, definitions } = await importMarkdownFragment( + "first\n\nsecond", + ); + assert.equal(blocks.length, 2); + assert.equal(definitions.length, 0); + assert.equal(blocks[0].type, "paragraph"); +}); + +test("importMarkdownFragment: `^[...]` footnote -> a definition + a remapped ref", async () => { + const { blocks, definitions } = await importMarkdownFragment( + "a claim^[the note]", + ); + assert.equal(definitions.length, 1); + const refs = findAll({ type: "doc", content: blocks }, "footnoteReference"); + assert.equal(refs.length, 1); + // The reference id must match the (remapped) definition id. + assert.equal(refs[0].attrs.id, definitions[0].attrs.id); + // The id is NOT the importer's sequential "fn-1" — it was remapped to a fresh + // uuid so it cannot collide with a page footnote of the same number. + assert.notEqual(refs[0].attrs.id, "fn-1"); +}); + +test("importMarkdownFragment: whitespace markdown imports to a single empty paragraph", async () => { + // The importer yields one empty paragraph for whitespace-only input (not zero + // blocks), so the fragment path returns that block. The client's XOR guard + // (markdown.trim() !== "") is what rejects an empty-string patch up front, so + // importMarkdownFragment never sees a truly empty string via patch/insert. + const { blocks, definitions } = await importMarkdownFragment(" \n "); + assert.equal(blocks.length, 1); + assert.equal(blocks[0].type, "paragraph"); + assert.equal(definitions.length, 0); +}); + +test("canBeDocChild: paragraph/heading/table are doc children; tableRow/cell are not", () => { + assert.equal(canBeDocChild("paragraph"), true); + assert.equal(canBeDocChild("heading"), true); + assert.equal(canBeDocChild("table"), true); + assert.equal(canBeDocChild("tableRow"), false); + assert.equal(canBeDocChild("tableCell"), false); + assert.equal(canBeDocChild("tableHeader"), false); + assert.equal(canBeDocChild("text"), false); + assert.equal(canBeDocChild(undefined), false); + assert.equal(canBeDocChild("notARealType"), false); +}); + +const cell = (attrs, text) => ({ + type: "tableCell", + attrs, + content: [{ type: "paragraph", content: [{ type: "text", text }] }], +}); + +test("findUnrepresentableTableAttrs: null for a plain paragraph and a simple table", () => { + assert.equal( + findUnrepresentableTableAttrs({ + type: "paragraph", + content: [{ type: "text", text: "x" }], + }), + null, + ); + const simpleTable = { + type: "table", + content: [ + { + type: "tableRow", + content: [cell({ colspan: 1, rowspan: 1 }, "a")], + }, + ], + }; + assert.equal(findUnrepresentableTableAttrs(simpleTable), null); +}); + +test("findUnrepresentableTableAttrs: flags colspan/rowspan/colwidth/backgroundColor", () => { + const mk = (attrs) => ({ + type: "table", + content: [{ type: "tableRow", content: [cell(attrs, "a")] }], + }); + assert.match(findUnrepresentableTableAttrs(mk({ colspan: 2 })), /colspan/); + assert.match(findUnrepresentableTableAttrs(mk({ rowspan: 2 })), /rowspan/); + assert.match( + findUnrepresentableTableAttrs(mk({ colwidth: [120] })), + /colwidth/, + ); + assert.match( + findUnrepresentableTableAttrs(mk({ backgroundColor: "#eee" })), + /backgroundColor/, + ); +}); + +test("findUnrepresentableTableAttrs: finds a span nested deep (table inside a callout)", () => { + const doc = { + type: "callout", + content: [ + { + type: "table", + content: [ + { type: "tableRow", content: [cell({ colspan: 3 }, "wide")] }, + ], + }, + ], + }; + assert.match(findUnrepresentableTableAttrs(doc), /colspan/); +}); diff --git a/packages/mcp/test/unit/tool-specs.test.mjs b/packages/mcp/test/unit/tool-specs.test.mjs index b3b84bd3..e94ee222 100644 --- a/packages/mcp/test/unit/tool-specs.test.mjs +++ b/packages/mcp/test/unit/tool-specs.test.mjs @@ -81,49 +81,68 @@ test("editPageText builder produces { pageId, edits } and drops the stale strip- assert.match(spec.description, /REFUSED into\s+failed\[\]/); }); -test("getNode builder produces exactly { pageId, nodeId }", () => { - const shape = SHARED_TOOL_SPECS.getNode.buildShape(z); - assert.deepEqual(Object.keys(shape).sort(), ["nodeId", "pageId"]); +// #413: getNode gained an optional `format` (markdown default / json opt-in). +test("getNode builder produces { pageId, nodeId, format? } with format optional", () => { + const spec = SHARED_TOOL_SPECS.getNode; + const shape = spec.buildShape(z); + assert.deepEqual(Object.keys(shape).sort(), ["format", "nodeId", "pageId"]); + const schema = z.object(shape); + // format is optional (markdown default lives in the client). + assert.doesNotThrow(() => schema.parse({ pageId: "p1", nodeId: "n1" })); + assert.doesNotThrow(() => + schema.parse({ pageId: "p1", nodeId: "n1", format: "json" }), + ); + assert.throws(() => + schema.parse({ pageId: "p1", nodeId: "n1", format: "yaml" }), + ); + // The description advertises the markdown default and the json opt-in. + assert.match(spec.description, /markdown/i); + assert.match(spec.description, /json/i); }); -test("patchNode spec exists, merges BOTH descriptions, builds { pageId, nodeId, node }", () => { +// #413: patchNode takes XOR { markdown | node } (both schema-optional). +test("patchNode spec exists, describes markdown+node XOR, builds { pageId, nodeId, markdown?, node? }", () => { const spec = SHARED_TOOL_SPECS.patchNode; assert.ok(spec, "patchNode spec missing"); assert.equal(spec.mcpName, "patchNode"); assert.equal(spec.inAppKey, "patchNode"); - // The canonical description must carry the key guidance from BOTH originals: - // - MCP-only: "WITHOUT resending the whole document" + the cheaper/safer note. - // - in-app-only: "keeps the same node id" + the "Reversible ... page history" - // framing the MCP copy lacked. - assert.match(spec.description, /WITHOUT resending the whole document/); - assert.match(spec.description, /Cheaper and safer/); - assert.match(spec.description, /keeps the same node id/i); + // The canonical description must carry the #413 guidance. + assert.match(spec.description, /WITHOUT/i); + assert.match(spec.description, /EXACTLY ONE of `markdown` or `node`/); + assert.match(spec.description, /RECOMMENDED/); + assert.match(spec.description, /keeps the same block id/i); assert.match(spec.description, /Reversible/i); assert.match(spec.description, /page history/i); const shape = spec.buildShape(z); - assert.deepEqual(Object.keys(shape).sort(), ["node", "nodeId", "pageId"]); - // A minimal valid input parses (node accepts an arbitrary object via z.any()). - const parsed = z.object(shape).parse({ + assert.deepEqual( + Object.keys(shape).sort(), + ["markdown", "node", "nodeId", "pageId"], + ); + // markdown and node are BOTH optional in the schema (XOR enforced at runtime). + const schema = z.object(shape); + const parsedMd = schema.parse({ pageId: "p1", nodeId: "n1", markdown: "hi" }); + assert.equal(parsedMd.markdown, "hi"); + const parsedNode = schema.parse({ pageId: "p1", nodeId: "n1", node: { type: "paragraph" }, }); - assert.equal(parsed.pageId, "p1"); - assert.equal(parsed.nodeId, "n1"); + assert.equal(parsedNode.pageId, "p1"); + // Neither given parses at the schema level (the client throws the XOR error). + assert.doesNotThrow(() => schema.parse({ pageId: "p1", nodeId: "n1" })); }); -test("insertNode spec exists, merges BOTH descriptions, builds the full anchor shape", () => { +// #413: insertNode also takes XOR { markdown | node } plus the anchor shape. +test("insertNode spec exists, describes markdown+node XOR, builds the full anchor+content shape", () => { const spec = SHARED_TOOL_SPECS.insertNode; assert.ok(spec, "insertNode spec missing"); assert.equal(spec.mcpName, "insertNode"); assert.equal(spec.inAppKey, "insertNode"); - // Canonical description must keep BOTH sides' nuance: - // - in-app-only: "EXACTLY ONE of anchorNodeId or anchorText" + "Reversible". - // - MCP-only: the table-structure (tableRow/tableCell) insertion guidance. assert.match(spec.description, /EXACTLY ONE of anchorNodeId or anchorText/); + assert.match(spec.description, /EXACTLY ONE of `markdown` or `node`/); assert.match(spec.description, /tableRow/); assert.match(spec.description, /append is top-level only/); assert.match(spec.description, /Reversible via page history/); @@ -131,15 +150,18 @@ test("insertNode spec exists, merges BOTH descriptions, builds the full anchor s const shape = spec.buildShape(z); assert.deepEqual( Object.keys(shape).sort(), - ["anchorNodeId", "anchorText", "node", "pageId", "position"], + ["anchorNodeId", "anchorText", "markdown", "node", "pageId", "position"], ); - // before/after/append are the only accepted positions; anchors are optional. + // before/after/append are the only accepted positions; markdown/node/anchors optional. const schema = z.object(shape); + assert.doesNotThrow(() => + schema.parse({ pageId: "p1", markdown: "hi", position: "append" }), + ); assert.doesNotThrow(() => schema.parse({ pageId: "p1", node: { type: "paragraph" }, position: "append" }), ); assert.throws(() => - schema.parse({ pageId: "p1", node: {}, position: "sideways" }), + schema.parse({ pageId: "p1", markdown: "x", position: "sideways" }), ); }); diff --git a/packages/prosemirror-markdown/src/lib/index.ts b/packages/prosemirror-markdown/src/lib/index.ts index 5cdbdb91..1529fa49 100644 --- a/packages/prosemirror-markdown/src/lib/index.ts +++ b/packages/prosemirror-markdown/src/lib/index.ts @@ -53,11 +53,14 @@ export { buildOutline, getNodeByRef, replaceNodeById, + replaceNodeByIdWithMany, + reassignCollidingBlockIds, deleteNodeById, sanitizeForYjs, findUnstorableAttr, findInvalidNode, insertNodeRelative, + insertNodesRelative, readTable, insertTableRow, deleteTableRow, diff --git a/packages/prosemirror-markdown/src/lib/node-ops.ts b/packages/prosemirror-markdown/src/lib/node-ops.ts index eadc5db0..6d88e3a3 100644 --- a/packages/prosemirror-markdown/src/lib/node-ops.ts +++ b/packages/prosemirror-markdown/src/lib/node-ops.ts @@ -217,6 +217,54 @@ export function replaceNodeById( return { doc: out, replaced }; } +/** + * Splice a SINGLE node whose `attrs.id === nodeId` with an ORDERED ARRAY of new + * nodes (a "1 -> N" replacement), anywhere in the tree. Used by the markdown + * patch path, where importing a markdown fragment can yield several blocks that + * must replace one existing block in place ("rewrite a section" in one call). + * + * Unlike `replaceNodeById` (which substitutes EVERY match), this walks to the + * FIRST match only and splices `newNodes` in its position, so ordering and the + * neighbouring blocks are preserved byte-for-byte. It deliberately does NOT + * touch further duplicates: the caller (#159 semantics) must have already + * verified the id is unambiguous via a `replaceNodeById` dry pass, so a single + * splice here is safe and every other block is untouched. + * + * Each entry of `newNodes` is deep-cloned so they never share references with + * each other or with the caller\'s array. Operates on a clone of `doc`; returns + * `{ doc, replaced }` where `replaced` is 1 when a match was spliced, else 0. + */ +export function replaceNodeByIdWithMany( + doc: any, + nodeId: string, + newNodes: any[], +): { doc: any; replaced: number } { + const out = clone(doc); + const fresh = Array.isArray(newNodes) ? newNodes.map((n) => clone(n)) : []; + let replaced = 0; + + // Walk to the FIRST match and splice the array in its place; stop afterwards. + const walkContent = (content: any[]): boolean => { + for (let i = 0; i < content.length; i++) { + const child = content[i]; + if (matchesId(child, nodeId)) { + content.splice(i, 1, ...fresh); + replaced = 1; + return true; + } + if (isObject(child) && Array.isArray(child.content)) { + if (walkContent(child.content)) return true; + } + } + return false; + }; + + if (isObject(out) && Array.isArray(out.content)) { + walkContent(out.content); + } + return { doc: out, replaced }; +} + /** * Remove EVERY node whose `attrs.id === nodeId` from its parent `content` * array, anywhere in the tree (recursive, including callouts and tables). @@ -725,6 +773,88 @@ export function insertNodeRelative( return { doc: out, inserted: false }; } +/** + * Insert an ORDERED ARRAY of nodes relative to an anchor, preserving their + * order. This is the multi-node twin of `insertNodeRelative`, used by the + * markdown insert path where importing a markdown fragment can yield several + * blocks that must land, in order, at one anchor. + * + * Semantics mirror `insertNodeRelative` exactly: + * - position "append": push every node onto the top-level `doc.content`. + * - position "before"/"after": splice every node into the anchor\'s parent + * `content` array immediately before / after it, keeping array order. + * + * The structural-table branch of `insertNodeRelative` is intentionally NOT + * duplicated here: a markdown fragment can never produce a bare tableRow/ + * tableCell/tableHeader (those are not expressible in markdown), so the markdown + * insert path only ever hands whole top-level blocks. Structural inserts stay on + * the single-node JSON path. An empty `nodes` array is a no-op that still + * reports `inserted:false` (nothing to place). + * + * Operates on a clone of `doc`; returns `{ doc, inserted }`. `inserted` is false + * when the anchor could not be resolved (doc returned unchanged apart from the + * clone) or when `nodes` is empty. + */ +export function insertNodesRelative( + doc: any, + nodes: any[], + opts: InsertOptions, +): { doc: any; inserted: boolean } { + const out = clone(doc); + const fresh = Array.isArray(nodes) ? nodes.map((n) => clone(n)) : []; + + if (!isObject(opts) || fresh.length === 0) { + return { doc: out, inserted: false }; + } + + // "append": push every node at the top level, in order. + if (opts.position === "append") { + if (isObject(out)) { + if (!Array.isArray(out.content)) out.content = []; + out.content.push(...fresh); + return { doc: out, inserted: true }; + } + return { doc: out, inserted: false }; + } + + const offset = opts.position === "after" ? 1 : 0; + + // Resolve by id anywhere in the tree: splice the whole array into the parent. + if (opts.anchorNodeId != null) { + let inserted = false; + const walkContent = (content: any[]): void => { + for (let i = 0; i < content.length; i++) { + const child = content[i]; + if (matchesId(child, opts.anchorNodeId as string)) { + content.splice(i + offset, 0, ...fresh); + inserted = true; + return; + } + if (isObject(child) && Array.isArray(child.content)) { + walkContent(child.content); + if (inserted) return; + } + } + }; + if (isObject(out) && Array.isArray(out.content)) { + walkContent(out.content); + } + return { doc: out, inserted }; + } + + // Resolve by text: only top-level doc.content blocks are scanned. Exact match + // wins; a markdown-stripped fallback is tried only on a miss. + if (opts.anchorText != null && isObject(out) && Array.isArray(out.content)) { + const i = findAnchorTextIndex(out.content, opts.anchorText); + if (i !== -1) { + out.content.splice(i + offset, 0, ...fresh); + return { doc: out, inserted: true }; + } + } + + return { doc: out, inserted: false }; +} + // =========================================================================== // Table editing helpers // @@ -773,6 +903,27 @@ function makeFreshId(used: Set): string { return id; } +/** + * Re-mint any top-level block id in `blocks` that already exists in `liveDoc`, + * so a 1 -> N splice cannot introduce a duplicate id. `skipIndex` (optional) is a + * block whose id is intentionally set (the patch path's first block inherits the + * target node's id) and must not be re-minted. Mutates `blocks` in place. + */ +export function reassignCollidingBlockIds( + liveDoc: any, + blocks: any[], + skipIndex?: number, +): void { + const used = new Set(); + collectIds(liveDoc, used); + blocks.forEach((b, i) => { + if (i === skipIndex || !isObject(b)) return; + if (!isObject(b.attrs)) b.attrs = {}; + if (b.attrs.id != null && used.has(b.attrs.id)) b.attrs.id = makeFreshId(used); + if (b.attrs.id != null) used.add(b.attrs.id); + }); +} + /** * Resolve a table reference against an ALREADY-CLONED doc and return the LIVE * table node (a reference inside `rootClone`, so the caller may mutate it) plus diff --git a/packages/prosemirror-markdown/test/node-ops-splice.test.ts b/packages/prosemirror-markdown/test/node-ops-splice.test.ts new file mode 100644 index 00000000..5e063a4f --- /dev/null +++ b/packages/prosemirror-markdown/test/node-ops-splice.test.ts @@ -0,0 +1,119 @@ +import { describe, expect, it } from "vitest"; +import { + replaceNodeByIdWithMany, + insertNodesRelative, +} from "../src/lib/node-ops.js"; + +// #413: the array-splice helpers used by the markdown patch/insert paths. +const p = (id: string, t: string): any => ({ + type: "paragraph", + attrs: { id }, + content: [{ type: "text", text: t }], +}); + +describe("replaceNodeByIdWithMany", () => { + it("splices N nodes in place of the first match, keeping neighbours byte-identical", () => { + const before = { type: "doc", content: [p("a", "A"), p("b", "B"), p("c", "C")] }; + const snap = JSON.parse(JSON.stringify(before)); + const { doc, replaced } = replaceNodeByIdWithMany(before, "b", [ + p("b1", "B1"), + p("b2", "B2"), + ]); + expect(replaced).toBe(1); + expect(doc.content.map((n: any) => n.attrs.id)).toEqual(["a", "b1", "b2", "c"]); + // Input never mutated. + expect(before).toEqual(snap); + // Neighbours byte-identical. + expect(doc.content[0]).toEqual(snap.content[0]); + expect(doc.content[3]).toEqual(snap.content[2]); + }); + + it("reaches a nested match (inside a callout) and splices there", () => { + const before = { + type: "doc", + content: [ + { type: "callout", attrs: { id: "co" }, content: [p("x", "X")] }, + ], + }; + const { doc, replaced } = replaceNodeByIdWithMany(before, "x", [ + p("x1", "X1"), + p("x2", "X2"), + ]); + expect(replaced).toBe(1); + expect(doc.content[0].content.map((n: any) => n.attrs.id)).toEqual(["x1", "x2"]); + }); + + it("only touches the FIRST duplicate (caller guards ambiguity)", () => { + const before = { type: "doc", content: [p("d", "1"), p("d", "2")] }; + const { doc, replaced } = replaceNodeByIdWithMany(before, "d", [p("n", "N")]); + expect(replaced).toBe(1); + // First replaced; the second duplicate survives untouched. + expect(doc.content.map((n: any) => n.attrs.id)).toEqual(["n", "d"]); + }); + + it("reports replaced:0 for no match, doc unchanged", () => { + const before = { type: "doc", content: [p("a", "A")] }; + const { doc, replaced } = replaceNodeByIdWithMany(before, "zzz", [p("n", "N")]); + expect(replaced).toBe(0); + expect(doc).toEqual(before); + }); +}); + +describe("insertNodesRelative", () => { + it("inserts an ordered array after an id anchor", () => { + const before = { type: "doc", content: [p("a", "A"), p("b", "B")] }; + const { doc, inserted } = insertNodesRelative( + before, + [p("n1", "N1"), p("n2", "N2")], + { position: "after", anchorNodeId: "a" }, + ); + expect(inserted).toBe(true); + expect(doc.content.map((n: any) => n.attrs.id)).toEqual(["a", "n1", "n2", "b"]); + }); + + it("inserts before an id anchor", () => { + const before = { type: "doc", content: [p("a", "A"), p("b", "B")] }; + const { doc } = insertNodesRelative(before, [p("n", "N")], { + position: "before", + anchorNodeId: "b", + }); + expect(doc.content.map((n: any) => n.attrs.id)).toEqual(["a", "n", "b"]); + }); + + it("appends an ordered array at the top level", () => { + const before = { type: "doc", content: [p("a", "A")] }; + const { doc } = insertNodesRelative(before, [p("n1", "N1"), p("n2", "N2")], { + position: "append", + }); + expect(doc.content.map((n: any) => n.attrs.id)).toEqual(["a", "n1", "n2"]); + }); + + it("resolves an anchor by top-level text", () => { + const before = { type: "doc", content: [p("a", "hello there"), p("b", "B")] }; + const { doc, inserted } = insertNodesRelative(before, [p("n", "N")], { + position: "after", + anchorText: "hello", + }); + expect(inserted).toBe(true); + expect(doc.content.map((n: any) => n.attrs.id)).toEqual(["a", "n", "b"]); + }); + + it("reports inserted:false when the anchor is missing", () => { + const before = { type: "doc", content: [p("a", "A")] }; + const { doc, inserted } = insertNodesRelative(before, [p("n", "N")], { + position: "after", + anchorNodeId: "missing", + }); + expect(inserted).toBe(false); + expect(doc).toEqual(before); + }); + + it("is a no-op for an empty node array", () => { + const before = { type: "doc", content: [p("a", "A")] }; + const { doc, inserted } = insertNodesRelative(before, [], { + position: "append", + }); + expect(inserted).toBe(false); + expect(doc).toEqual(before); + }); +});