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 e5fcd5ba..f6d38487 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 @@ -370,12 +370,29 @@ export class AiChatToolsService { createComment: tool({ description: - 'Add a comment to a page, or reply to an existing top-level comment ' + - '(one level only — the backend rejects replies to replies). ' + - 'Reversible via the comment UI.', + 'Add an INLINE comment to a page, or reply to an existing top-level ' + + 'comment (one level only — the backend rejects replies to replies). ' + + 'The comment is anchored inline to the given exact `selection` text ' + + '(which gets highlighted); page-level comments are NOT supported. A ' + + "new top-level comment REQUIRES a `selection`. Replies inherit the " + + "parent's anchor and take no selection. If the call fails with a " + + '"selection not found" error, retry with a corrected EXACT selection ' + + 'copied verbatim from a single paragraph/block. Reversible via the ' + + 'comment UI.', inputSchema: z.object({ pageId: z.string().describe('The id of the page to comment on.'), content: z.string().describe('The comment body as Markdown.'), + selection: z + .string() + .min(1) + .max(250) + .optional() + .describe( + 'EXACT contiguous text from a SINGLE paragraph/block to anchor ' + + '(highlight) the comment on (<=250 chars, avoid spanning across ' + + 'formatting boundaries). Required for a new top-level comment; ' + + 'omit only when replying via parentCommentId.', + ), parentCommentId: z .string() .optional() @@ -384,14 +401,22 @@ export class AiChatToolsService { 'of replies only).', ), }), - execute: async ({ pageId, content, parentCommentId }) => { + execute: async ({ pageId, content, selection, parentCommentId }) => { // createComment(pageId, content, type, selection?, parentCommentId?). - // Page-type comment (no inline selection); replies inherit the anchor. + // Top-level comments are inline and must carry a selection to anchor + // on; replies inherit the parent's anchor (no selection). Throwing + // here surfaces a tool error to the model (Vercel `ai` SDK) so the + // agent retries with a better selection — do not catch/suppress it. + if (!parentCommentId && (!selection || !selection.trim())) { + throw new Error( + "createComment requires a 'selection' (exact text to anchor on) for a new top-level comment.", + ); + } const result = await client.createComment( pageId, content, - 'page', - undefined, + 'inline', + selection, parentCommentId, ); const data = (result?.data ?? {}) as { id?: string }; diff --git a/packages/mcp/build/client.js b/packages/mcp/build/client.js index eefb8add..a825dd03 100644 --- a/packages/mcp/build/client.js +++ b/packages/mcp/build/client.js @@ -16,6 +16,7 @@ import { withPageLock } from "./lib/page-lock.js"; import { applyTextEdits, } from "./lib/json-edit.js"; import { getCollabToken, performLogin } from "./lib/auth-utils.js"; import { diffDocs, summarizeChange } from "./lib/diff.js"; +import { applyAnchorInDoc, canAnchorInDoc, } from "./lib/comment-anchor.js"; import { blockText, walk, getList, insertMarkerAfter, setCalloutRange, noteItem, mdToInlineNodes, commentsToFootnotes, } from "./lib/transforms.js"; import vm from "node:vm"; // Supported image types, kept as two lookup tables so both a local file @@ -1513,17 +1514,61 @@ export class DocmostClient { success: true, }; } - /** Create a page-level or inline comment; content is markdown. */ + /** + * Create an inline comment anchored to its `selection` text, or a reply. + * + * Top-level comments (no `parentCommentId`) are ALWAYS inline and MUST carry a + * `selection`: the `type` argument is kept for interface compatibility but the + * effective type is coerced to "inline". The selection has to anchor in the + * document; if it cannot, the comment is rolled back and an error is thrown so + * the caller is forced to supply a proper inline selection rather than leaving + * an orphan, unanchored comment behind. Replies (parentCommentId set) inherit + * their parent's anchor: they take NO selection and are not anchored. + */ async createComment(pageId, content, type = "page", selection, parentCommentId) { await this.ensureAuthenticated(); + const isReply = !!parentCommentId; + // Only top-level comments are inline-anchored, so they are stored as + // "inline". Replies carry no inline selection, so they keep the historical + // general ("page") type — both backward-compatible and semantically correct. + // The `type` argument is kept for interface compatibility; createComment + // normalizes the effective type internally, so callers may pass "inline". + const effectiveType = isReply ? "page" : "inline"; + if (!isReply && (!selection || !selection.trim())) { + throw new Error("create_comment: an inline 'selection' (exact text to anchor on) is required for a top-level comment"); + } + // For a top-level comment, fail BEFORE creating anything when the selection + // is not present in the persisted document — this avoids leaving an orphan + // comment + notification behind. A read failure (network) is non-fatal: the + // live anchor step below still enforces the anchoring invariant. + if (!isReply && selection) { + try { + const page = await this.getPageJson(pageId); + if (!canAnchorInDoc(page.content, selection)) { + throw new Error("create_comment: could not find the selection text in the page to anchor the comment. " + + "Provide the EXACT contiguous text from a single paragraph/block (<=250 chars)."); + } + } + catch (e) { + // Rethrow our own "not found" error; swallow read/network errors so the + // live anchor step can still try (and enforce) the anchoring. + if (e instanceof Error && + e.message.startsWith("create_comment: could not find the selection")) { + throw e; + } + if (process.env.DEBUG) { + console.error("Pre-check getPageJson failed; deferring to live anchor step:", e); + } + } + } // Convert through the full Docmost schema (consistent with page paths) const jsonContent = await markdownToProseMirror(content); const payload = { pageId, content: JSON.stringify(jsonContent), - type, + type: effectiveType, }; - if (selection) + if (!isReply && selection) payload.selection = selection; if (parentCommentId) payload.parentCommentId = parentCommentId; @@ -1536,96 +1581,72 @@ export class DocmostClient { data: filterComment(comment, markdown), success: true, }; + // Replies inherit the parent's anchor: no selection, no anchoring. + if (isReply) { + return result; + } // Anchor the comment in the document. The /comments/create API records the // comment + its `selection` text, but it does NOT insert the comment MARK // into the page content, so without this the inline comment has no - // highlight/anchor and is not clickable. Only top-level inline comments are - // anchored: replies (parentCommentId set) inherit their parent's anchor, - // and page-type comments have no text range. - if (type === "inline" && selection && !parentCommentId && comment?.id) { - const newCommentId = comment.id; - let anchored = false; - try { - const collabToken = await this.getCollabTokenWithReauth(); - const mutation = await mutatePageContent(pageId, collabToken, this.apiUrl, (liveDoc) => { - const doc = liveDoc && liveDoc.type === "doc" - ? liveDoc - : { type: "doc", content: [] }; - // Find the FIRST text node containing the selection text, then - // split it into before / marked / after, copying the node's - // existing marks onto all three parts and adding the comment mark - // only to the middle part. Returns true once a match is wrapped. - const wrapInFirstMatch = (nodes, depth) => { - const MAX_DEPTH = 200; - if (depth > MAX_DEPTH || !Array.isArray(nodes)) - return false; - for (let i = 0; i < nodes.length; i++) { - const n = nodes[i]; - if (!n || typeof n !== "object") - continue; - if (n.type === "text" && - typeof n.text === "string" && - n.text.includes(selection)) { - const idx = n.text.indexOf(selection); - const before = n.text.slice(0, idx); - const middleText = selection; - const after = n.text.slice(idx + selection.length); - const baseMarks = Array.isArray(n.marks) ? n.marks : []; - // Drop any pre-existing comment mark from the marks applied to - // the middle fragment so it ends up with exactly one comment - // mark (the new one) rather than two. Other fragments and the - // base marks list are left untouched. - const middleBaseMarks = baseMarks.filter((m) => !(m && m.type === "comment")); - const commentMark = { - type: "comment", - // The comment mark schema declares both commentId and - // resolved; include resolved:false for completeness. - attrs: { commentId: newCommentId, resolved: false }, - }; - const parts = []; - if (before.length > 0) { - parts.push({ ...n, text: before, marks: [...baseMarks] }); - } - parts.push({ - ...n, - text: middleText, - marks: [...middleBaseMarks, commentMark], - }); - if (after.length > 0) { - parts.push({ ...n, text: after, marks: [...baseMarks] }); - } - nodes.splice(i, 1, ...parts); - return true; - } - if (Array.isArray(n.content)) { - if (wrapInFirstMatch(n.content, depth + 1)) - return true; - } - } - return false; - }; - if (Array.isArray(doc.content) && wrapInFirstMatch(doc.content, 0)) { - anchored = true; - return doc; - } - // Selection text not found: do NOT fail (the comment already - // exists). Abort the write so nothing changes. - return null; - }); - result.verify = mutation.verify; - } - catch (e) { - // The comment record already exists; an anchoring failure must not turn - // a successful create into an error. Report anchored:false instead. - if (process.env.DEBUG) { - console.error("Failed to anchor inline comment mark:", e); - } - anchored = false; - } - result.anchored = anchored; + // highlight/anchor and is not clickable. If anchoring fails the comment is + // rolled back (deleted) and an error is thrown — never an orphan comment. + const newCommentId = comment.id; + // Guard: a create response without an id would mean writing a comment mark + // with commentId: undefined and a later delete of a falsy id. We have no id + // to roll back here (nothing was created with an id), so just fail loudly. + if (!newCommentId) { + throw new Error("create_comment: the server returned no comment id, so the comment could not be anchored"); } + let anchored = false; + try { + const collabToken = await this.getCollabTokenWithReauth(); + const mutation = await mutatePageContent(pageId, collabToken, this.apiUrl, (liveDoc) => { + const doc = liveDoc && liveDoc.type === "doc" + ? liveDoc + : { type: "doc", content: [] }; + if (applyAnchorInDoc(doc, selection, newCommentId)) { + anchored = true; + return doc; + } + // Selection text not found in the LIVE document: abort the write. The + // rollback + throw below turns this into a hard error. + return null; + }); + result.verify = mutation.verify; + } + catch (e) { + // The comment record already exists; roll it back so we never leave an + // orphan, then rethrow the original anchoring error. + await this.safeDeleteComment(newCommentId); + throw e; + } + if (!anchored) { + // Mutation aborted because the selection was not found in the live + // document. Roll back the comment and surface a hard error. + await this.safeDeleteComment(newCommentId); + throw new Error("create_comment: failed to anchor the comment (selection not found in the live document); the comment was rolled back"); + } + result.anchored = true; return result; } + /** + * Best-effort rollback of a just-created comment. Swallows any delete failure + * (logging under DEBUG) so a failed cleanup never masks the original error. + */ + async safeDeleteComment(commentId) { + // Defense in depth: never call the delete API with a falsy id — there is + // nothing to roll back, and deleteComment(undefined) would hit a bad route. + if (!commentId) + return; + try { + await this.deleteComment(commentId); + } + catch (delErr) { + if (process.env.DEBUG) { + console.error("Failed to roll back comment after anchoring error:", delErr); + } + } + } async updateComment(commentId, content) { await this.ensureAuthenticated(); const jsonContent = await markdownToProseMirror(content); diff --git a/packages/mcp/build/index.js b/packages/mcp/build/index.js index b0c20413..7f258a19 100644 --- a/packages/mcp/build/index.js +++ b/packages/mcp/build/index.js @@ -27,7 +27,7 @@ const VERSION = packageJson.version; // --- Modern McpServer Implementation --- // Editing guide surfaced to MCP clients in the initialize result so they can // pick the right tool by intent and avoid resending whole documents. -const SERVER_INSTRUCTIONS = "Docmost editing guide — choose the tool by intent: fix wording/typos/numbers (text inside blocks) -> edit_page_text (no node id needed). Change ONE block (paragraph/heading/callout/table cell/etc.) structurally -> patch_node (address by attrs.id from get_page_json). Add a block -> insert_node (before/after a block by attrs.id or by anchor text, or append). Remove a block -> delete_node (by attrs.id). Images -> insert_image (add an image from a web URL) / replace_image (swap an existing image for one from a web URL). New page -> create_page (Markdown). Bulk/structural rewrite or nodes without an id -> update_page_json (full ProseMirror replace; prefer the granular tools above to avoid resending the whole ~100KB+ document). Copy/replace a page's whole content from another page (server-side, no document through the model) -> copy_page_content. Rename a page (title only) -> rename_page. Read -> get_page (Markdown, lossy) or get_page_json (lossless ProseMirror with block ids). Comments -> create_comment (an inline comment anchors to its selection text), list_comments, update_comment, delete_comment, check_new_comments. Tip: read block ids via get_page_json, then use patch_node/insert_node/delete_node so you never resend the full document. " + +const SERVER_INSTRUCTIONS = "Docmost editing guide — choose the tool by intent: fix wording/typos/numbers (text inside blocks) -> edit_page_text (no node id needed). Change ONE block (paragraph/heading/callout/table cell/etc.) structurally -> patch_node (address by attrs.id from get_page_json). Add a block -> insert_node (before/after a block by attrs.id or by anchor text, or append). Remove a block -> delete_node (by attrs.id). Images -> insert_image (add an image from a web URL) / replace_image (swap an existing image for one from a web URL). New page -> create_page (Markdown). Bulk/structural rewrite or nodes without an id -> update_page_json (full ProseMirror replace; prefer the granular tools above to avoid resending the whole ~100KB+ document). Copy/replace a page's whole content from another page (server-side, no document through the model) -> copy_page_content. Rename a page (title only) -> rename_page. Read -> get_page (Markdown, lossy) or get_page_json (lossless ProseMirror with block ids). Comments -> create_comment (always inline; requires an EXACT selection — the contiguous text to anchor/highlight on; fails rather than leaving an unanchored comment), list_comments, update_comment, delete_comment, check_new_comments. Tip: read block ids via get_page_json, then use patch_node/insert_node/delete_node so you never resend the full document. " + "Complex/scripted rewrite (multiple coordinated edits, footnotes, renumbering) -> docmost_transform: write a JS `(doc, ctx) => doc` transform, preview the diff with dryRun (default), then apply with dryRun:false; ctx.helpers includes commentsToFootnotes for turning inline comments into numbered footnotes. " + "Review what changed -> diff_page_versions (compare a historyId to current, or two history versions). See a page's saved versions -> list_page_history. Undo a bad edit -> restore_page_version (writes a past version back as current; itself revertible). " + "Lossless markdown round-trip (download, edit, re-upload, incl. comment anchors) -> export_page_markdown / import_page_markdown."; @@ -508,28 +508,35 @@ export function createDocmostMcpServer(config) { }); // Tool: create_comment server.registerTool("create_comment", { - description: "Create a new comment on a page. Content is provided as Markdown and " + - "automatically converted to the required format.", + description: "Create a new comment on a page. The comment is ALWAYS inline and is " + + "anchored to (highlights) its `selection` text — there are no page-level " + + "comments. Content is provided as Markdown and automatically converted. " + + "A top-level comment REQUIRES an exact `selection`; if the selection " + + "cannot be found in the page the call fails (no orphan comment is left). " + + "Replies (parentCommentId set) inherit the parent's anchor and take no " + + "selection.", inputSchema: { pageId: z.string().describe("ID of the page to comment on"), content: z.string().min(1).describe("Comment content in Markdown format"), - type: z - .enum(["page", "inline"]) - .optional() - .describe("Comment type: 'page' for general page comment (default), 'inline' for text selection comment"), selection: z .string() + .min(1) // Enforce the documented 250-char cap to match the description above. .max(250) .optional() - .describe("For an inline comment, the EXACT text in the page to anchor/highlight the comment on (the first occurrence of this text is wrapped in a comment mark). Max 250 chars. Required when type is 'inline'."), + .describe("EXACT contiguous text from a single paragraph/block to anchor the " + + "comment on (<=250 chars). Required for a top-level comment; omit " + + "only when replying via parentCommentId."), parentCommentId: z .string() .optional() .describe("Parent comment ID to create a reply (max 2 nesting levels)"), }, - }, async ({ pageId, content, type, selection, parentCommentId }) => { - const result = await docmostClient.createComment(pageId, content, type || "page", selection, parentCommentId); + }, async ({ pageId, content, selection, parentCommentId }) => { + if (!parentCommentId && (!selection || !selection.trim())) { + throw new Error("create_comment: a 'selection' (exact text to anchor on) is required for a top-level comment; omit it only when replying via parentCommentId."); + } + const result = await docmostClient.createComment(pageId, content, "inline", selection, parentCommentId); return jsonContent(result); }); // Tool: update_comment diff --git a/packages/mcp/build/lib/comment-anchor.js b/packages/mcp/build/lib/comment-anchor.js new file mode 100644 index 00000000..50e113b2 --- /dev/null +++ b/packages/mcp/build/lib/comment-anchor.js @@ -0,0 +1,239 @@ +/** + * Inline-comment anchoring against a ProseMirror document. + * + * Docmost stores an inline comment's highlight as a `comment` MARK on the + * document text (`{ type: "comment", attrs: { commentId, resolved } }`); the + * `/comments/create` API only records the comment row + its `selection` text and + * does NOT insert that mark, so the anchor has to be written into the page + * content separately. This module finds where a selection lives in the document + * and splices the comment mark across the matched range. + * + * Matching has to be robust because the agent supplies the selection as plain + * text while the document stores rich inline content: a selection can span + * several adjacent text nodes (inline code / bold / links each become their own + * text node), and the document may use smart/typographic quotes, dash variants, + * non-breaking spaces, or collapsed runs of whitespace that the agent typed as + * ASCII quotes/hyphens/single spaces. We therefore normalize both sides before + * comparing and match across maximal runs of consecutive text nodes within a + * single block, while mapping every normalized character back to its raw index + * so the mark lands on the exact original characters. + */ +/** Typographic double-quote variants mapped to ASCII `"`. */ +const DOUBLE_QUOTES = "«»„“”‟〝〞＂"; +/** Typographic single-quote/apostrophe variants mapped to ASCII `'`. */ +const SINGLE_QUOTES = "‘’‚‛"; +/** Dash variants mapped to ASCII `-`. */ +const DASHES = "–—―−‐‑‒"; +/** Guard against pathological/cyclic documents in the depth-first walk. */ +const MAX_DEPTH = 200; +/** The comment mark Docmost stores on anchored text. */ +function makeCommentMark(commentId) { + // The comment mark schema declares both commentId and resolved; include + // resolved:false for completeness so the stored mark matches the editor's. + return { type: "comment", attrs: { commentId, resolved: false } }; +} +/** True for any character we collapse/replace with a single normal space. */ +function isWhitespaceChar(ch) { + // Regular ASCII whitespace plus the special spaces called out in the spec: + // nbsp, narrow nbsp, en/em/thin/hair/figure spaces, etc. \s covers tab and + // newline; the explicit code points cover the non-breaking variants \s misses + // in some engines, so list them for determinism. + return (/\s/.test(ch) || + ch === " " || // no-break space + ch === " " || // figure space + ch === " " || // narrow no-break space + ch === " " || // thin space + ch === " " || // hair space + ch === " " || // en space + ch === " " // em space + ); +} +/** + * Normalize a string for matching and return both the normalized text and a + * `map` where `map[i]` is the index into the ORIGINAL `s` of the i-th + * normalized character. + * + * Rules: map smart quotes / dashes / special spaces to their ASCII forms, + * collapse any run of whitespace to a SINGLE space (whose map entry points at + * the FIRST raw whitespace char of the run), and DO NOT lowercase (anchoring is + * case-sensitive to match the exact document text). + */ +export function normalizeForMatch(s) { + let norm = ""; + const map = []; + let i = 0; + while (i < s.length) { + const ch = s[i]; + if (isWhitespaceChar(ch)) { + // Collapse the whole whitespace run to one space mapped to the run start. + const runStart = i; + while (i < s.length && isWhitespaceChar(s[i])) + i++; + norm += " "; + map.push(runStart); + continue; + } + let mapped = ch; + if (DOUBLE_QUOTES.indexOf(ch) !== -1) + mapped = '"'; + else if (SINGLE_QUOTES.indexOf(ch) !== -1) + mapped = "'"; + else if (DASHES.indexOf(ch) !== -1) + mapped = "-"; + norm += mapped; + map.push(i); + i++; + } + return { norm, map }; +} +/** + * Find a selection inside a SINGLE block's direct `content` array. + * + * Builds maximal runs of consecutive `text` nodes (any non-text inline node, + * e.g. a mention, breaks the run), normalizes each run and the selection the + * same way, then searches each run for the normalized selection. Returns the + * child/offset range of the FIRST matching run, or `null` if none match. + */ +export function findAnchorInBlock(blockContent, selection) { + if (!Array.isArray(blockContent)) + return null; + const normSelObj = normalizeForMatch(selection); + // Trim leading/trailing spaces on the NORMALIZED selection only. + const normSel = normSelObj.norm.trim(); + if (normSel.length === 0) + return null; + let i = 0; + while (i < blockContent.length) { + const node = blockContent[i]; + if (!node || typeof node !== "object" || node.type !== "text") { + i++; + continue; + } + // Accumulate a maximal run of consecutive text nodes. + let rawRun = ""; + const rawToChild = []; + let j = i; + while (j < blockContent.length) { + const n = blockContent[j]; + if (!n || typeof n !== "object" || n.type !== "text") + break; + const text = typeof n.text === "string" ? n.text : ""; + for (let k = 0; k < text.length; k++) { + rawToChild.push({ childIdx: j, offset: k }); + } + rawRun += text; + j++; + } + // Try to match within this run. + const { norm, map } = normalizeForMatch(rawRun); + const idx = norm.indexOf(normSel); + if (idx !== -1) { + const rawStart = map[idx]; + const rawEndExclusive = idx + normSel.length < map.length + ? map[idx + normSel.length] + : rawRun.length; + const startLoc = rawToChild[rawStart]; + // rawEndExclusive points at the raw char AFTER the match; the last matched + // raw char is at rawEndExclusive-1, so endOffset is its offset + 1. + const lastLoc = rawToChild[rawEndExclusive - 1]; + return { + startChild: startLoc.childIdx, + startOffset: startLoc.offset, + endChild: lastLoc.childIdx, + endOffset: lastLoc.offset + 1, + }; + } + // No match in this run: continue scanning AFTER it. + i = j > i ? j : i + 1; + } + return null; +} +/** + * Depth-first, document-order check for whether `selection` can be anchored + * anywhere in `doc`. At each node with an array `content`, first try to match + * within that node's own content, then recurse into children that themselves + * have a `content` array. + */ +export function canAnchorInDoc(doc, selection) { + const visit = (node, depth) => { + if (depth > MAX_DEPTH || !node || typeof node !== "object") + return false; + if (!Array.isArray(node.content)) + return false; + if (findAnchorInBlock(node.content, selection)) + return true; + for (const child of node.content) { + if (child && typeof child === "object" && Array.isArray(child.content)) { + if (visit(child, depth + 1)) + return true; + } + } + return false; + }; + return visit(doc, 0); +} +/** + * Split the matched text nodes and splice the comment mark across the range. + * `blockContent` is mutated IN PLACE. `match.startChild..endChild` are all text + * nodes (guaranteed by findAnchorInBlock building runs of text nodes). + */ +function spliceCommentMark(blockContent, match, commentId) { + const { startChild, startOffset, endChild, endOffset } = match; + const commentMark = makeCommentMark(commentId); + const fragments = []; + for (let k = startChild; k <= endChild; k++) { + const n = blockContent[k]; + const text = typeof n.text === "string" ? n.text : ""; + const sliceStart = k === startChild ? startOffset : 0; + const sliceEnd = k === endChild ? endOffset : text.length; + const before = k === startChild ? text.slice(0, startOffset) : ""; + const marked = text.slice(sliceStart, sliceEnd); + const after = k === endChild ? text.slice(endOffset) : ""; + // Process per-node so each node's OWN marks/attrs are preserved. + const ownMarks = Array.isArray(n.marks) ? n.marks : []; + // Drop any pre-existing comment mark from the marked fragment so it ends up + // with exactly one comment mark (the new one) rather than two. + const markedBaseMarks = ownMarks.filter((m) => !(m && m.type === "comment")); + if (before.length > 0) { + fragments.push({ ...n, text: before, marks: [...ownMarks] }); + } + if (marked.length > 0) { + fragments.push({ + ...n, + text: marked, + marks: [...markedBaseMarks, commentMark], + }); + } + if (after.length > 0) { + fragments.push({ ...n, text: after, marks: [...ownMarks] }); + } + } + blockContent.splice(startChild, endChild - startChild + 1, ...fragments); +} +/** + * Depth-first (same order as canAnchorInDoc) over `doc`; on the FIRST block + * whose content matches `selection`, splice the comment mark across the matched + * range in place and return true. Returns false (and does NOT mutate) when no + * block matches. + */ +export function applyAnchorInDoc(doc, selection, commentId) { + const visit = (node, depth) => { + if (depth > MAX_DEPTH || !node || typeof node !== "object") + return false; + if (!Array.isArray(node.content)) + return false; + const match = findAnchorInBlock(node.content, selection); + if (match) { + spliceCommentMark(node.content, match, commentId); + return true; + } + for (const child of node.content) { + if (child && typeof child === "object" && Array.isArray(child.content)) { + if (visit(child, depth + 1)) + return true; + } + } + return false; + }; + return visit(doc, 0); +} diff --git a/packages/mcp/src/client.ts b/packages/mcp/src/client.ts index 0f2dc495..a2e60336 100644 --- a/packages/mcp/src/client.ts +++ b/packages/mcp/src/client.ts @@ -48,6 +48,10 @@ import { } from "./lib/json-edit.js"; import { getCollabToken, performLogin } from "./lib/auth-utils.js"; import { diffDocs, summarizeChange } from "./lib/diff.js"; +import { + applyAnchorInDoc, + canAnchorInDoc, +} from "./lib/comment-anchor.js"; import { blockText, walk, @@ -1912,7 +1916,17 @@ export class DocmostClient { }; } - /** Create a page-level or inline comment; content is markdown. */ + /** + * Create an inline comment anchored to its `selection` text, or a reply. + * + * Top-level comments (no `parentCommentId`) are ALWAYS inline and MUST carry a + * `selection`: the `type` argument is kept for interface compatibility but the + * effective type is coerced to "inline". The selection has to anchor in the + * document; if it cannot, the comment is rolled back and an error is thrown so + * the caller is forced to supply a proper inline selection rather than leaving + * an orphan, unanchored comment behind. Replies (parentCommentId set) inherit + * their parent's anchor: they take NO selection and are not anchored. + */ async createComment( pageId: string, content: string, @@ -1921,14 +1935,59 @@ export class DocmostClient { parentCommentId?: string, ) { await this.ensureAuthenticated(); + + const isReply = !!parentCommentId; + // Only top-level comments are inline-anchored, so they are stored as + // "inline". Replies carry no inline selection, so they keep the historical + // general ("page") type — both backward-compatible and semantically correct. + // The `type` argument is kept for interface compatibility; createComment + // normalizes the effective type internally, so callers may pass "inline". + const effectiveType: "page" | "inline" = isReply ? "page" : "inline"; + if (!isReply && (!selection || !selection.trim())) { + throw new Error( + "create_comment: an inline 'selection' (exact text to anchor on) is required for a top-level comment", + ); + } + + // For a top-level comment, fail BEFORE creating anything when the selection + // is not present in the persisted document — this avoids leaving an orphan + // comment + notification behind. A read failure (network) is non-fatal: the + // live anchor step below still enforces the anchoring invariant. + if (!isReply && selection) { + try { + const page = await this.getPageJson(pageId); + if (!canAnchorInDoc(page.content, selection)) { + throw new Error( + "create_comment: could not find the selection text in the page to anchor the comment. " + + "Provide the EXACT contiguous text from a single paragraph/block (<=250 chars).", + ); + } + } catch (e) { + // Rethrow our own "not found" error; swallow read/network errors so the + // live anchor step can still try (and enforce) the anchoring. + if ( + e instanceof Error && + e.message.startsWith("create_comment: could not find the selection") + ) { + throw e; + } + if (process.env.DEBUG) { + console.error( + "Pre-check getPageJson failed; deferring to live anchor step:", + e, + ); + } + } + } + // Convert through the full Docmost schema (consistent with page paths) const jsonContent = await markdownToProseMirror(content); const payload: Record = { pageId, content: JSON.stringify(jsonContent), - type, + type: effectiveType, }; - if (selection) payload.selection = selection; + if (!isReply && selection) payload.selection = selection; if (parentCommentId) payload.parentCommentId = parentCommentId; const response = await this.client.post("/comments/create", payload); @@ -1941,109 +2000,87 @@ export class DocmostClient { success: true, }; + // Replies inherit the parent's anchor: no selection, no anchoring. + if (isReply) { + return result; + } + // Anchor the comment in the document. The /comments/create API records the // comment + its `selection` text, but it does NOT insert the comment MARK // into the page content, so without this the inline comment has no - // highlight/anchor and is not clickable. Only top-level inline comments are - // anchored: replies (parentCommentId set) inherit their parent's anchor, - // and page-type comments have no text range. - if (type === "inline" && selection && !parentCommentId && comment?.id) { - const newCommentId: string = comment.id; - let anchored = false; - try { - const collabToken = await this.getCollabTokenWithReauth(); - const mutation = await mutatePageContent( - pageId, - collabToken, - this.apiUrl, - (liveDoc) => { - const doc = - liveDoc && liveDoc.type === "doc" - ? liveDoc - : { type: "doc", content: [] }; - - // Find the FIRST text node containing the selection text, then - // split it into before / marked / after, copying the node's - // existing marks onto all three parts and adding the comment mark - // only to the middle part. Returns true once a match is wrapped. - const wrapInFirstMatch = ( - nodes: any[], - depth: number, - ): boolean => { - const MAX_DEPTH = 200; - if (depth > MAX_DEPTH || !Array.isArray(nodes)) return false; - for (let i = 0; i < nodes.length; i++) { - const n = nodes[i]; - if (!n || typeof n !== "object") continue; - if ( - n.type === "text" && - typeof n.text === "string" && - n.text.includes(selection) - ) { - const idx = n.text.indexOf(selection); - const before = n.text.slice(0, idx); - const middleText = selection; - const after = n.text.slice(idx + selection.length); - const baseMarks = Array.isArray(n.marks) ? n.marks : []; - // Drop any pre-existing comment mark from the marks applied to - // the middle fragment so it ends up with exactly one comment - // mark (the new one) rather than two. Other fragments and the - // base marks list are left untouched. - const middleBaseMarks = baseMarks.filter( - (m: any) => !(m && m.type === "comment"), - ); - const commentMark = { - type: "comment", - // The comment mark schema declares both commentId and - // resolved; include resolved:false for completeness. - attrs: { commentId: newCommentId, resolved: false }, - }; - const parts: any[] = []; - if (before.length > 0) { - parts.push({ ...n, text: before, marks: [...baseMarks] }); - } - parts.push({ - ...n, - text: middleText, - marks: [...middleBaseMarks, commentMark], - }); - if (after.length > 0) { - parts.push({ ...n, text: after, marks: [...baseMarks] }); - } - nodes.splice(i, 1, ...parts); - return true; - } - if (Array.isArray(n.content)) { - if (wrapInFirstMatch(n.content, depth + 1)) return true; - } - } - return false; - }; - - if (Array.isArray(doc.content) && wrapInFirstMatch(doc.content, 0)) { - anchored = true; - return doc; - } - // Selection text not found: do NOT fail (the comment already - // exists). Abort the write so nothing changes. - return null; - }, - ); - result.verify = mutation.verify; - } catch (e) { - // The comment record already exists; an anchoring failure must not turn - // a successful create into an error. Report anchored:false instead. - if (process.env.DEBUG) { - console.error("Failed to anchor inline comment mark:", e); - } - anchored = false; - } - result.anchored = anchored; + // highlight/anchor and is not clickable. If anchoring fails the comment is + // rolled back (deleted) and an error is thrown — never an orphan comment. + const newCommentId: string = comment.id; + // Guard: a create response without an id would mean writing a comment mark + // with commentId: undefined and a later delete of a falsy id. We have no id + // to roll back here (nothing was created with an id), so just fail loudly. + if (!newCommentId) { + throw new Error( + "create_comment: the server returned no comment id, so the comment could not be anchored", + ); + } + let anchored = false; + try { + const collabToken = await this.getCollabTokenWithReauth(); + const mutation = await mutatePageContent( + pageId, + collabToken, + this.apiUrl, + (liveDoc) => { + const doc = + liveDoc && liveDoc.type === "doc" + ? liveDoc + : { type: "doc", content: [] }; + if (applyAnchorInDoc(doc, selection as string, newCommentId)) { + anchored = true; + return doc; + } + // Selection text not found in the LIVE document: abort the write. The + // rollback + throw below turns this into a hard error. + return null; + }, + ); + result.verify = mutation.verify; + } catch (e) { + // The comment record already exists; roll it back so we never leave an + // orphan, then rethrow the original anchoring error. + await this.safeDeleteComment(newCommentId); + throw e; } + if (!anchored) { + // Mutation aborted because the selection was not found in the live + // document. Roll back the comment and surface a hard error. + await this.safeDeleteComment(newCommentId); + throw new Error( + "create_comment: failed to anchor the comment (selection not found in the live document); the comment was rolled back", + ); + } + + result.anchored = true; return result; } + /** + * Best-effort rollback of a just-created comment. Swallows any delete failure + * (logging under DEBUG) so a failed cleanup never masks the original error. + */ + private async safeDeleteComment(commentId: string): Promise { + // Defense in depth: never call the delete API with a falsy id — there is + // nothing to roll back, and deleteComment(undefined) would hit a bad route. + if (!commentId) return; + try { + await this.deleteComment(commentId); + } catch (delErr) { + if (process.env.DEBUG) { + console.error( + "Failed to roll back comment after anchoring error:", + delErr, + ); + } + } + } + async updateComment(commentId: string, content: string) { await this.ensureAuthenticated(); const jsonContent = await markdownToProseMirror(content); diff --git a/packages/mcp/src/index.ts b/packages/mcp/src/index.ts index 54e40bca..51d1489b 100644 --- a/packages/mcp/src/index.ts +++ b/packages/mcp/src/index.ts @@ -38,7 +38,7 @@ const VERSION = packageJson.version; // Editing guide surfaced to MCP clients in the initialize result so they can // pick the right tool by intent and avoid resending whole documents. const SERVER_INSTRUCTIONS = - "Docmost editing guide — choose the tool by intent: fix wording/typos/numbers (text inside blocks) -> edit_page_text (no node id needed). Change ONE block (paragraph/heading/callout/table cell/etc.) structurally -> patch_node (address by attrs.id from get_page_json). Add a block -> insert_node (before/after a block by attrs.id or by anchor text, or append). Remove a block -> delete_node (by attrs.id). Images -> insert_image (add an image from a web URL) / replace_image (swap an existing image for one from a web URL). New page -> create_page (Markdown). Bulk/structural rewrite or nodes without an id -> update_page_json (full ProseMirror replace; prefer the granular tools above to avoid resending the whole ~100KB+ document). Copy/replace a page's whole content from another page (server-side, no document through the model) -> copy_page_content. Rename a page (title only) -> rename_page. Read -> get_page (Markdown, lossy) or get_page_json (lossless ProseMirror with block ids). Comments -> create_comment (an inline comment anchors to its selection text), list_comments, update_comment, delete_comment, check_new_comments. Tip: read block ids via get_page_json, then use patch_node/insert_node/delete_node so you never resend the full document. " + + "Docmost editing guide — choose the tool by intent: fix wording/typos/numbers (text inside blocks) -> edit_page_text (no node id needed). Change ONE block (paragraph/heading/callout/table cell/etc.) structurally -> patch_node (address by attrs.id from get_page_json). Add a block -> insert_node (before/after a block by attrs.id or by anchor text, or append). Remove a block -> delete_node (by attrs.id). Images -> insert_image (add an image from a web URL) / replace_image (swap an existing image for one from a web URL). New page -> create_page (Markdown). Bulk/structural rewrite or nodes without an id -> update_page_json (full ProseMirror replace; prefer the granular tools above to avoid resending the whole ~100KB+ document). Copy/replace a page's whole content from another page (server-side, no document through the model) -> copy_page_content. Rename a page (title only) -> rename_page. Read -> get_page (Markdown, lossy) or get_page_json (lossless ProseMirror with block ids). Comments -> create_comment (always inline; requires an EXACT selection — the contiguous text to anchor/highlight on; fails rather than leaving an unanchored comment), list_comments, update_comment, delete_comment, check_new_comments. Tip: read block ids via get_page_json, then use patch_node/insert_node/delete_node so you never resend the full document. " + "Complex/scripted rewrite (multiple coordinated edits, footnotes, renumbering) -> docmost_transform: write a JS `(doc, ctx) => doc` transform, preview the diff with dryRun (default), then apply with dryRun:false; ctx.helpers includes commentsToFootnotes for turning inline comments into numbered footnotes. " + "Review what changed -> diff_page_versions (compare a historyId to current, or two history versions). See a page's saved versions -> list_page_history. Undo a bad edit -> restore_page_version (writes a past version back as current; itself revertible). " + "Lossless markdown round-trip (download, edit, re-upload, incl. comment anchors) -> export_page_markdown / import_page_markdown."; @@ -713,24 +713,26 @@ server.registerTool( "create_comment", { description: - "Create a new comment on a page. Content is provided as Markdown and " + - "automatically converted to the required format.", + "Create a new comment on a page. The comment is ALWAYS inline and is " + + "anchored to (highlights) its `selection` text — there are no page-level " + + "comments. Content is provided as Markdown and automatically converted. " + + "A top-level comment REQUIRES an exact `selection`; if the selection " + + "cannot be found in the page the call fails (no orphan comment is left). " + + "Replies (parentCommentId set) inherit the parent's anchor and take no " + + "selection.", inputSchema: { pageId: z.string().describe("ID of the page to comment on"), content: z.string().min(1).describe("Comment content in Markdown format"), - type: z - .enum(["page", "inline"]) - .optional() - .describe( - "Comment type: 'page' for general page comment (default), 'inline' for text selection comment", - ), selection: z .string() + .min(1) // Enforce the documented 250-char cap to match the description above. .max(250) .optional() .describe( - "For an inline comment, the EXACT text in the page to anchor/highlight the comment on (the first occurrence of this text is wrapped in a comment mark). Max 250 chars. Required when type is 'inline'.", + "EXACT contiguous text from a single paragraph/block to anchor the " + + "comment on (<=250 chars). Required for a top-level comment; omit " + + "only when replying via parentCommentId.", ), parentCommentId: z .string() @@ -738,11 +740,16 @@ server.registerTool( .describe("Parent comment ID to create a reply (max 2 nesting levels)"), }, }, - async ({ pageId, content, type, selection, parentCommentId }) => { + async ({ pageId, content, selection, parentCommentId }) => { + if (!parentCommentId && (!selection || !selection.trim())) { + throw new Error( + "create_comment: a 'selection' (exact text to anchor on) is required for a top-level comment; omit it only when replying via parentCommentId.", + ); + } const result = await docmostClient.createComment( pageId, content, - type || "page", + "inline", selection, parentCommentId, ); diff --git a/packages/mcp/src/lib/comment-anchor.ts b/packages/mcp/src/lib/comment-anchor.ts new file mode 100644 index 00000000..79dbb469 --- /dev/null +++ b/packages/mcp/src/lib/comment-anchor.ts @@ -0,0 +1,272 @@ +/** + * Inline-comment anchoring against a ProseMirror document. + * + * Docmost stores an inline comment's highlight as a `comment` MARK on the + * document text (`{ type: "comment", attrs: { commentId, resolved } }`); the + * `/comments/create` API only records the comment row + its `selection` text and + * does NOT insert that mark, so the anchor has to be written into the page + * content separately. This module finds where a selection lives in the document + * and splices the comment mark across the matched range. + * + * Matching has to be robust because the agent supplies the selection as plain + * text while the document stores rich inline content: a selection can span + * several adjacent text nodes (inline code / bold / links each become their own + * text node), and the document may use smart/typographic quotes, dash variants, + * non-breaking spaces, or collapsed runs of whitespace that the agent typed as + * ASCII quotes/hyphens/single spaces. We therefore normalize both sides before + * comparing and match across maximal runs of consecutive text nodes within a + * single block, while mapping every normalized character back to its raw index + * so the mark lands on the exact original characters. + */ + +/** Typographic double-quote variants mapped to ASCII `"`. */ +const DOUBLE_QUOTES = "«»„“”‟〝〞＂"; +/** Typographic single-quote/apostrophe variants mapped to ASCII `'`. */ +const SINGLE_QUOTES = "‘’‚‛"; +/** Dash variants mapped to ASCII `-`. */ +const DASHES = "–—―−‐‑‒"; + +/** Guard against pathological/cyclic documents in the depth-first walk. */ +const MAX_DEPTH = 200; + +/** The comment mark Docmost stores on anchored text. */ +function makeCommentMark(commentId: string): any { + // The comment mark schema declares both commentId and resolved; include + // resolved:false for completeness so the stored mark matches the editor's. + return { type: "comment", attrs: { commentId, resolved: false } }; +} + +/** True for any character we collapse/replace with a single normal space. */ +function isWhitespaceChar(ch: string): boolean { + // Regular ASCII whitespace plus the special spaces called out in the spec: + // nbsp, narrow nbsp, en/em/thin/hair/figure spaces, etc. \s covers tab and + // newline; the explicit code points cover the non-breaking variants \s misses + // in some engines, so list them for determinism. + return ( + /\s/.test(ch) || + ch === " " || // no-break space + ch === " " || // figure space + ch === " " || // narrow no-break space + ch === " " || // thin space + ch === " " || // hair space + ch === " " || // en space + ch === " " // em space + ); +} + +/** + * Normalize a string for matching and return both the normalized text and a + * `map` where `map[i]` is the index into the ORIGINAL `s` of the i-th + * normalized character. + * + * Rules: map smart quotes / dashes / special spaces to their ASCII forms, + * collapse any run of whitespace to a SINGLE space (whose map entry points at + * the FIRST raw whitespace char of the run), and DO NOT lowercase (anchoring is + * case-sensitive to match the exact document text). + */ +export function normalizeForMatch(s: string): { norm: string; map: number[] } { + let norm = ""; + const map: number[] = []; + let i = 0; + while (i < s.length) { + const ch = s[i]; + if (isWhitespaceChar(ch)) { + // Collapse the whole whitespace run to one space mapped to the run start. + const runStart = i; + while (i < s.length && isWhitespaceChar(s[i])) i++; + norm += " "; + map.push(runStart); + continue; + } + let mapped = ch; + if (DOUBLE_QUOTES.indexOf(ch) !== -1) mapped = '"'; + else if (SINGLE_QUOTES.indexOf(ch) !== -1) mapped = "'"; + else if (DASHES.indexOf(ch) !== -1) mapped = "-"; + norm += mapped; + map.push(i); + i++; + } + return { norm, map }; +} + +/** Descriptor of a matched range inside one block's `content` array. */ +export interface AnchorMatch { + startChild: number; + startOffset: number; + endChild: number; + endOffset: number; +} + +/** Per-raw-char location inside a run: which child node and offset within it. */ +interface RawLoc { + childIdx: number; + offset: number; +} + +/** + * Find a selection inside a SINGLE block's direct `content` array. + * + * Builds maximal runs of consecutive `text` nodes (any non-text inline node, + * e.g. a mention, breaks the run), normalizes each run and the selection the + * same way, then searches each run for the normalized selection. Returns the + * child/offset range of the FIRST matching run, or `null` if none match. + */ +export function findAnchorInBlock( + blockContent: any[], + selection: string, +): AnchorMatch | null { + if (!Array.isArray(blockContent)) return null; + + const normSelObj = normalizeForMatch(selection); + // Trim leading/trailing spaces on the NORMALIZED selection only. + const normSel = normSelObj.norm.trim(); + if (normSel.length === 0) return null; + + let i = 0; + while (i < blockContent.length) { + const node = blockContent[i]; + if (!node || typeof node !== "object" || node.type !== "text") { + i++; + continue; + } + // Accumulate a maximal run of consecutive text nodes. + let rawRun = ""; + const rawToChild: RawLoc[] = []; + let j = i; + while (j < blockContent.length) { + const n = blockContent[j]; + if (!n || typeof n !== "object" || n.type !== "text") break; + const text = typeof n.text === "string" ? n.text : ""; + for (let k = 0; k < text.length; k++) { + rawToChild.push({ childIdx: j, offset: k }); + } + rawRun += text; + j++; + } + + // Try to match within this run. + const { norm, map } = normalizeForMatch(rawRun); + const idx = norm.indexOf(normSel); + if (idx !== -1) { + const rawStart = map[idx]; + const rawEndExclusive = + idx + normSel.length < map.length + ? map[idx + normSel.length] + : rawRun.length; + const startLoc = rawToChild[rawStart]; + // rawEndExclusive points at the raw char AFTER the match; the last matched + // raw char is at rawEndExclusive-1, so endOffset is its offset + 1. + const lastLoc = rawToChild[rawEndExclusive - 1]; + return { + startChild: startLoc.childIdx, + startOffset: startLoc.offset, + endChild: lastLoc.childIdx, + endOffset: lastLoc.offset + 1, + }; + } + + // No match in this run: continue scanning AFTER it. + i = j > i ? j : i + 1; + } + return null; +} + +/** + * Depth-first, document-order check for whether `selection` can be anchored + * anywhere in `doc`. At each node with an array `content`, first try to match + * within that node's own content, then recurse into children that themselves + * have a `content` array. + */ +export function canAnchorInDoc(doc: any, selection: string): boolean { + const visit = (node: any, depth: number): boolean => { + if (depth > MAX_DEPTH || !node || typeof node !== "object") return false; + if (!Array.isArray(node.content)) return false; + if (findAnchorInBlock(node.content, selection)) return true; + for (const child of node.content) { + if (child && typeof child === "object" && Array.isArray(child.content)) { + if (visit(child, depth + 1)) return true; + } + } + return false; + }; + return visit(doc, 0); +} + +/** + * Split the matched text nodes and splice the comment mark across the range. + * `blockContent` is mutated IN PLACE. `match.startChild..endChild` are all text + * nodes (guaranteed by findAnchorInBlock building runs of text nodes). + */ +function spliceCommentMark( + blockContent: any[], + match: AnchorMatch, + commentId: string, +): void { + const { startChild, startOffset, endChild, endOffset } = match; + const commentMark = makeCommentMark(commentId); + const fragments: any[] = []; + + for (let k = startChild; k <= endChild; k++) { + const n = blockContent[k]; + const text: string = typeof n.text === "string" ? n.text : ""; + const sliceStart = k === startChild ? startOffset : 0; + const sliceEnd = k === endChild ? endOffset : text.length; + + const before = k === startChild ? text.slice(0, startOffset) : ""; + const marked = text.slice(sliceStart, sliceEnd); + const after = k === endChild ? text.slice(endOffset) : ""; + + // Process per-node so each node's OWN marks/attrs are preserved. + const ownMarks: any[] = Array.isArray(n.marks) ? n.marks : []; + // Drop any pre-existing comment mark from the marked fragment so it ends up + // with exactly one comment mark (the new one) rather than two. + const markedBaseMarks = ownMarks.filter( + (m: any) => !(m && m.type === "comment"), + ); + + if (before.length > 0) { + fragments.push({ ...n, text: before, marks: [...ownMarks] }); + } + if (marked.length > 0) { + fragments.push({ + ...n, + text: marked, + marks: [...markedBaseMarks, commentMark], + }); + } + if (after.length > 0) { + fragments.push({ ...n, text: after, marks: [...ownMarks] }); + } + } + + blockContent.splice(startChild, endChild - startChild + 1, ...fragments); +} + +/** + * Depth-first (same order as canAnchorInDoc) over `doc`; on the FIRST block + * whose content matches `selection`, splice the comment mark across the matched + * range in place and return true. Returns false (and does NOT mutate) when no + * block matches. + */ +export function applyAnchorInDoc( + doc: any, + selection: string, + commentId: string, +): boolean { + const visit = (node: any, depth: number): boolean => { + if (depth > MAX_DEPTH || !node || typeof node !== "object") return false; + if (!Array.isArray(node.content)) return false; + const match = findAnchorInBlock(node.content, selection); + if (match) { + spliceCommentMark(node.content, match, commentId); + return true; + } + for (const child of node.content) { + if (child && typeof child === "object" && Array.isArray(child.content)) { + if (visit(child, depth + 1)) return true; + } + } + return false; + }; + return visit(doc, 0); +} diff --git a/packages/mcp/test/mock/create-comment.test.mjs b/packages/mcp/test/mock/create-comment.test.mjs new file mode 100644 index 00000000..c0d6859e --- /dev/null +++ b/packages/mcp/test/mock/create-comment.test.mjs @@ -0,0 +1,231 @@ +// Mock-HTTP orchestration tests for DocmostClient.createComment. createComment +// is inline-only and anchored: a top-level comment REQUIRES a selection that +// can be anchored in the document (a failure rolls the comment back and throws), +// while a reply inherits its parent's anchor and is stored as the historical +// "page" type. These tests stand a local http.createServer in for Docmost and +// only mock plain-HTTP routes — they deliberately avoid the live anchoring step +// (the Hocuspocus collab WebSocket) by either short-circuiting BEFORE creation +// (cases 1 and 2) or exercising the reply path that skips anchoring (case 3). +import { test, after } from "node:test"; +import assert from "node:assert/strict"; +import http from "node:http"; +import { DocmostClient } from "../../build/client.js"; + +// Read a request body to completion (drain the stream and parse JSON when used). +function readBody(req) { + return new Promise((resolve) => { + let raw = ""; + req.on("data", (chunk) => { + raw += chunk; + }); + req.on("end", () => resolve(raw)); + }); +} + +// Start an http server bound to an ephemeral port and resolve once it is +// listening, returning the server plus the api base URL the client should use. +function startServer(handler) { + return new Promise((resolve) => { + const server = http.createServer(handler); + server.listen(0, "127.0.0.1", () => { + const { port } = server.address(); + resolve({ server, baseURL: `http://127.0.0.1:${port}/api` }); + }); + }); +} + +function closeServer(server) { + return new Promise((resolve) => server.close(resolve)); +} + +// JSON helper. +function sendJson(res, status, obj, extraHeaders = {}) { + res.writeHead(status, { "Content-Type": "application/json", ...extraHeaders }); + res.end(JSON.stringify(obj)); +} + +// Track every server so the after() hook can guarantee nothing is left open. +const openServers = []; +async function spawn(handler) { + const { server, baseURL } = await startServer(handler); + openServers.push(server); + return { server, baseURL }; +} + +after(async () => { + await Promise.all(openServers.map((s) => closeServer(s))); +}); + +// ----------------------------------------------------------------------------- +// 1) Top-level comment without a selection throws and creates nothing. +// ----------------------------------------------------------------------------- +test("a top-level comment without a selection throws and never POSTs /comments/create", async () => { + let createCalls = 0; + + const { baseURL } = await spawn(async (req, res) => { + await readBody(req); + if (req.url === "/api/auth/login") { + sendJson(res, 200, { success: true }, { + "Set-Cookie": "authToken=t; Path=/; HttpOnly", + }); + return; + } + if (req.url === "/api/comments/create") { + createCalls++; + sendJson(res, 200, { data: { id: "should-not-happen" } }); + return; + } + sendJson(res, 404, { message: "not found" }); + }); + + const client = new DocmostClient(baseURL, "user@example.com", "pw"); + + await assert.rejects( + () => client.createComment("page-1", "body", "inline", undefined), + /selection/i, + "a missing selection must reject with a 'selection required' error", + ); + assert.equal( + createCalls, + 0, + "/comments/create must NEVER be called when the selection is missing", + ); +}); + +// ----------------------------------------------------------------------------- +// 2) Top-level comment whose selection is absent from the page throws BEFORE +// creating anything (the getPageJson / /pages/info pre-check short-circuits). +// ----------------------------------------------------------------------------- +test("a top-level comment whose selection is absent from the page throws before creating", async () => { + let createCalls = 0; + let infoCalls = 0; + + const { baseURL } = await spawn(async (req, res) => { + await readBody(req); + if (req.url === "/api/auth/login") { + sendJson(res, 200, { success: true }, { + "Set-Cookie": "authToken=t; Path=/; HttpOnly", + }); + return; + } + if (req.url === "/api/pages/info") { + infoCalls++; + // A page whose body does NOT contain the requested selection text. + sendJson(res, 200, { + data: { + id: "page-1", + slugId: "slug-1", + title: "Page", + spaceId: "sp-1", + content: { + type: "doc", + content: [ + { + type: "paragraph", + content: [{ type: "text", text: "hello world" }], + }, + ], + }, + }, + }); + return; + } + if (req.url === "/api/comments/create") { + createCalls++; + sendJson(res, 200, { data: { id: "should-not-happen" } }); + return; + } + sendJson(res, 404, { message: "not found" }); + }); + + const client = new DocmostClient(baseURL, "user@example.com", "pw"); + + await assert.rejects( + () => + client.createComment( + "page-1", + "body", + "inline", + "this text is not present", + ), + /could not find the selection/i, + "an unanchorable selection must reject with a 'could not find the selection' error", + ); + assert.ok(infoCalls >= 1, "the pre-check must read the page via /pages/info"); + assert.equal( + createCalls, + 0, + "/comments/create must NEVER be called when the pre-check fails", + ); +}); + +// ----------------------------------------------------------------------------- +// 3) A reply (parentCommentId set) creates successfully WITHOUT a selection, +// WITHOUT anchoring, and is stored as type "page" — the pre-check/anchoring +// (and thus /pages/info) is skipped entirely. +// ----------------------------------------------------------------------------- +test("a reply creates without selection or anchoring and is stored as type 'page'", async () => { + let createPayload = null; + let infoCalls = 0; + + const { baseURL } = await spawn(async (req, res) => { + const raw = await readBody(req); + if (req.url === "/api/auth/login") { + sendJson(res, 200, { success: true }, { + "Set-Cookie": "authToken=t; Path=/; HttpOnly", + }); + return; + } + if (req.url === "/api/pages/info") { + infoCalls++; + sendJson(res, 200, { data: { id: "page-1", content: { type: "doc", content: [] } } }); + return; + } + if (req.url === "/api/comments/create") { + createPayload = JSON.parse(raw); + sendJson(res, 200, { + data: { + id: "c-reply-1", + content: createPayload.content, + parentCommentId: createPayload.parentCommentId, + type: createPayload.type, + }, + }); + return; + } + sendJson(res, 404, { message: "not found" }); + }); + + const client = new DocmostClient(baseURL, "user@example.com", "pw"); + + const result = await client.createComment( + "page-1", + "reply body", + "inline", + undefined, + "parent-123", + ); + + assert.equal(result.success, true, "a reply must resolve successfully"); + assert.ok(createPayload, "/comments/create must have been called"); + assert.equal( + createPayload.parentCommentId, + "parent-123", + "the reply payload must carry the parentCommentId", + ); + assert.equal( + createPayload.type, + "page", + "a reply must be stored as the historical 'page' type, not 'inline'", + ); + assert.equal( + "selection" in createPayload, + false, + "a reply payload must NOT carry a selection field", + ); + assert.equal( + infoCalls, + 0, + "a reply must skip the pre-check/anchoring (no /pages/info read)", + ); +}); diff --git a/packages/mcp/test/unit/comment-anchor.test.mjs b/packages/mcp/test/unit/comment-anchor.test.mjs new file mode 100644 index 00000000..490cba96 --- /dev/null +++ b/packages/mcp/test/unit/comment-anchor.test.mjs @@ -0,0 +1,210 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; + +import { + normalizeForMatch, + findAnchorInBlock, + canAnchorInDoc, + applyAnchorInDoc, +} from "../../build/lib/comment-anchor.js"; + +const COMMENT_ID = "cmt-123"; + +/** Find the (single) comment mark on a node, or null. */ +function commentMark(node) { + const marks = Array.isArray(node.marks) ? node.marks : []; + return marks.find((m) => m && m.type === "comment") || null; +} + +/** Build a one-paragraph doc with the given inline content array. */ +function paragraphDoc(content) { + return { type: "doc", content: [{ type: "paragraph", content }] }; +} + +test("normalizeForMatch maps a normalized char to its first raw index in a whitespace run", () => { + const { norm, map } = normalizeForMatch("a b"); // two spaces collapse to one + assert.equal(norm, "a b"); + // norm[1] is the single space; it maps to the FIRST raw whitespace (index 1). + assert.equal(map[1], 1); + assert.equal(map[2], 3); // 'b' is at raw index 3 +}); + +test("simple single-text-node match inserts the comment mark with correct id", () => { + const doc = paragraphDoc([{ type: "text", text: "Hello brave world" }]); + const ok = applyAnchorInDoc(doc, "brave", COMMENT_ID); + assert.equal(ok, true); + + const parts = doc.content[0].content; + // "Hello " | "brave" | " world" + assert.equal(parts.length, 3); + assert.equal(parts[0].text, "Hello "); + assert.equal(commentMark(parts[0]), null); + assert.equal(parts[1].text, "brave"); + const m = commentMark(parts[1]); + assert.ok(m, "marked fragment carries a comment mark"); + assert.equal(m.attrs.commentId, COMMENT_ID); + assert.equal(m.attrs.resolved, false); + assert.equal(parts[2].text, " world"); + assert.equal(commentMark(parts[2]), null); +}); + +test("match spanning two adjacent plain text nodes preserves base marks", () => { + const doc = paragraphDoc([ + { type: "text", text: "запуска ", marks: [{ type: "italic" }] }, + { type: "text", text: "перед блоком", marks: [{ type: "italic" }] }, + ]); + const ok = applyAnchorInDoc(doc, "запуска перед", COMMENT_ID); + assert.equal(ok, true); + + const parts = doc.content[0].content; + // "запуска " (marked) | "перед" (marked) | " блоком" (after) + assert.equal(parts.length, 3); + assert.equal(parts[0].text, "запуска "); + assert.equal(parts[1].text, "перед"); + assert.equal(parts[2].text, " блоком"); + + // Marked fragments keep the italic base mark AND get exactly one comment mark. + for (const p of [parts[0], parts[1]]) { + assert.ok(p.marks.some((m) => m.type === "italic")); + const cm = p.marks.filter((m) => m.type === "comment"); + assert.equal(cm.length, 1); + assert.equal(cm[0].attrs.commentId, COMMENT_ID); + } + // The trailing fragment keeps its italic mark and has no comment mark. + assert.ok(parts[2].marks.some((m) => m.type === "italic")); + assert.equal(commentMark(parts[2]), null); +}); + +test("match across an inline-code boundary preserves the code mark on the middle fragment", () => { + const doc = paragraphDoc([ + { type: "text", text: "run " }, + { type: "text", text: "qemu", marks: [{ type: "code" }] }, + { type: "text", text: " now" }, + ]); + const ok = applyAnchorInDoc(doc, "run qemu now", COMMENT_ID); + assert.equal(ok, true); + + const parts = doc.content[0].content; + // All three nodes are fully inside the match -> three marked fragments. + assert.equal(parts.length, 3); + assert.equal(parts[0].text, "run "); + assert.equal(parts[1].text, "qemu"); + assert.equal(parts[2].text, " now"); + + // Every fragment carries exactly one comment mark. + for (const p of parts) { + const cm = p.marks.filter((m) => m.type === "comment"); + assert.equal(cm.length, 1); + assert.equal(cm[0].attrs.commentId, COMMENT_ID); + } + // The middle fragment retains its code mark. + assert.ok(parts[1].marks.some((m) => m.type === "code")); +}); + +test("normalization matches smart quotes / em-dash / nbsp / collapsed spaces", () => { + // Document uses « », an em-dash, a non-breaking space, and a double space. + const docText = "He said «hello world» — done"; + const doc = paragraphDoc([{ type: "text", text: docText }]); + + // Selection typed with ASCII quotes, single spaces and a hyphen. + const selection = '"hello world" - done'; + assert.equal(canAnchorInDoc(doc, selection), true); + + const ok = applyAnchorInDoc(doc, selection, COMMENT_ID); + assert.equal(ok, true); + + const parts = doc.content[0].content; + const marked = parts.filter((p) => commentMark(p)); + assert.equal(marked.length, 1); + // The marked raw text starts at the « and ends at the trailing "done". + assert.ok(marked[0].text.startsWith("«hello")); + assert.ok(marked[0].text.endsWith("done")); +}); + +test("canAnchorInDoc/applyAnchorInDoc fail (and do not mutate) when selection absent", () => { + const doc = paragraphDoc([{ type: "text", text: "Hello brave world" }]); + const snapshot = JSON.stringify(doc); + + assert.equal(canAnchorInDoc(doc, "missing text"), false); + assert.equal(applyAnchorInDoc(doc, "missing text", COMMENT_ID), false); + // Document is unchanged after a failed apply. + assert.equal(JSON.stringify(doc), snapshot); +}); + +test("before/after fragments retain original marks; marked has exactly one comment mark", () => { + const doc = paragraphDoc([ + { type: "text", text: "abc def ghi", marks: [{ type: "bold" }] }, + ]); + const ok = applyAnchorInDoc(doc, "def", COMMENT_ID); + assert.equal(ok, true); + + const parts = doc.content[0].content; + assert.equal(parts.length, 3); + // before "abc " and after " ghi" keep the bold mark, no comment mark. + assert.deepEqual(parts[0].marks, [{ type: "bold" }]); + assert.deepEqual(parts[2].marks, [{ type: "bold" }]); + // marked "def" keeps bold and has exactly one comment mark. + assert.ok(parts[1].marks.some((m) => m.type === "bold")); + assert.equal(parts[1].marks.filter((m) => m.type === "comment").length, 1); +}); + +test("findAnchorInBlock returns child/offset descriptor for a multi-node run", () => { + const blockContent = [ + { type: "text", text: "ab" }, + { type: "text", text: "cdef" }, + ]; + const match = findAnchorInBlock(blockContent, "bcd"); + assert.deepEqual(match, { + startChild: 0, + startOffset: 1, + endChild: 1, + endOffset: 2, + }); +}); + +test("a pre-existing comment mark on matched text is replaced (single comment mark)", () => { + const doc = paragraphDoc([ + { + type: "text", + text: "Hello world", + marks: [{ type: "comment", attrs: { commentId: "old", resolved: false } }], + }, + ]); + const ok = applyAnchorInDoc(doc, "Hello world", COMMENT_ID); + assert.equal(ok, true); + const parts = doc.content[0].content; + assert.equal(parts.length, 1); + const cm = parts[0].marks.filter((m) => m.type === "comment"); + assert.equal(cm.length, 1); + assert.equal(cm[0].attrs.commentId, COMMENT_ID); +}); + +test("anchoring works inside a nested block (e.g. list item) via DFS recursion", () => { + const doc = { + type: "doc", + content: [ + { + type: "bulletList", + content: [ + { + type: "listItem", + content: [ + { + type: "paragraph", + content: [{ type: "text", text: "nested target here" }], + }, + ], + }, + ], + }, + ], + }; + assert.equal(canAnchorInDoc(doc, "target"), true); + const ok = applyAnchorInDoc(doc, "target", COMMENT_ID); + assert.equal(ok, true); + const para = + doc.content[0].content[0].content[0].content; + const marked = para.filter((p) => commentMark(p)); + assert.equal(marked.length, 1); + assert.equal(marked[0].text, "target"); +});