From a09935aa294c16dd9479ff456db9b98b4ededdf1 Mon Sep 17 00:00:00 2001 From: agent_coder Date: Sat, 11 Jul 2026 18:19:10 +0300 Subject: [PATCH] =?UTF-8?q?fix(mcp):=20resolved-=D1=8F=D0=BA=D0=BE=D1=80?= =?UTF-8?q?=D1=8F=20=D0=BF=D0=B5=D1=80=D0=B5=D0=B6=D0=B8=D0=B2=D0=B0=D1=8E?= =?UTF-8?q?=D1=82=20=D0=BF=D0=BE=D0=BB=D0=BD=D1=8B=D0=B9=20markdown-write?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Read-путь прячет resolved comment-анкоры (#337), поэтому markdown, который агент шлёт в updatePageMarkdown, их уже не содержит — наивный full-write стирал ВСЕ resolved comment-марки (потеря данных). Активные комментарии переживают round-trip сами (read отдаёт их ), а resolved — нет. Правка: в write-пути (updatePageContentRealtime) пере-прививаем resolved- марки из ЖИВОГО дока на совпадающие текстовые диапазоны свежеимпортированного тела, механикой анкоринга comment-anchor. spliceCommentMark обобщён на произвольную марку; добавлены applyCommentMarkInDoc (сохраняет resolved:true + attrs), collectResolvedCommentSpans и regraftResolvedComments (чистая, не мутирует входы). Спан, чей текст агент изменил/удалил, просто не переанкорится и отбрасывается (он и так resolved). first-occurrence-семантика как у остального анкоринга. Проверено: 6 новых наблюдаемых тестов + весь MCP unit-suite (691) зелёные. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/mcp/src/lib/collaboration.ts | 9 +- packages/mcp/src/lib/comment-anchor.ts | 115 +++++++++++++++++- .../unit/regraft-resolved-comments.test.mjs | 107 ++++++++++++++++ 3 files changed, 227 insertions(+), 4 deletions(-) create mode 100644 packages/mcp/test/unit/regraft-resolved-comments.test.mjs diff --git a/packages/mcp/src/lib/collaboration.ts b/packages/mcp/src/lib/collaboration.ts index 8b6fc1a3..e5219e25 100644 --- a/packages/mcp/src/lib/collaboration.ts +++ b/packages/mcp/src/lib/collaboration.ts @@ -23,6 +23,7 @@ import { } from "@docmost/prosemirror-markdown"; import { canonicalizeFootnotes } from "./footnote-canonicalize.js"; import { normalizeAndMergeFootnotes } from "./footnote-normalize-merge.js"; +import { regraftResolvedComments } from "./comment-anchor.js"; import { VerifyReport } from "./diff.js"; import { acquireCollabSession } from "./collab-session.js"; @@ -338,6 +339,12 @@ export async function updatePageContentRealtime( pageId, collabToken, baseUrl, - () => tiptapJson, + // #493: an agent read HIDES resolved-comment anchors (#337), so the markdown + // it sends here no longer carries them — a naive full rewrite would erase + // every resolved comment mark. Re-graft the resolved marks from the LIVE doc + // onto the matching text in the freshly-imported body. Active comments are + // untouched (they ride through the markdown themselves); a resolved span whose + // text the agent changed simply does not re-anchor and is dropped. + (liveDoc) => regraftResolvedComments(liveDoc, tiptapJson), ); } diff --git a/packages/mcp/src/lib/comment-anchor.ts b/packages/mcp/src/lib/comment-anchor.ts index b42cd780..78b887a5 100644 --- a/packages/mcp/src/lib/comment-anchor.ts +++ b/packages/mcp/src/lib/comment-anchor.ts @@ -312,10 +312,9 @@ export function canAnchorInDoc(doc: any, selection: string): boolean { function spliceCommentMark( blockContent: any[], match: AnchorMatch, - commentId: string, + commentMark: any, ): void { const { startChild, startOffset, endChild, endOffset } = match; - const commentMark = makeCommentMark(commentId); const fragments: any[] = []; for (let k = startChild; k <= endChild; k++) { @@ -451,6 +450,22 @@ export function applyAnchorInDoc( doc: any, selection: string, commentId: string, +): boolean { + return applyCommentMarkInDoc(doc, selection, makeCommentMark(commentId)); +} + +/** + * Core of {@link applyAnchorInDoc}, but splices an ARBITRARY comment mark object + * (not just a fresh `{ commentId, resolved:false }`) across the first matching + * range. This lets a caller re-apply a mark that carries `resolved:true` and any + * other stored attrs. Depth-first (same order as canAnchorInDoc); mutates in + * place on the first matching block and returns true, else returns false without + * mutating. + */ +export function applyCommentMarkInDoc( + doc: any, + selection: string, + commentMark: any, ): boolean { const { selection: effective, found } = resolveAnchorSelection(doc, selection); if (!found) return false; @@ -459,7 +474,7 @@ export function applyAnchorInDoc( if (!Array.isArray(node.content)) return false; const match = findAnchorInBlock(node.content, effective); if (match) { - spliceCommentMark(node.content, match, commentId); + spliceCommentMark(node.content, match, commentMark); return true; } for (const child of node.content) { @@ -471,3 +486,97 @@ export function applyAnchorInDoc( }; return visit(doc, 0); } + +/** A resolved inline-comment span lifted from a doc: its mark + anchored text. */ +export interface ResolvedCommentSpan { + commentId: string; + /** The full comment mark (carrying `resolved:true` + any stored attrs). */ + mark: any; + /** The concatenated raw text the mark spans — used as the re-anchor selection. */ + text: string; +} + +/** True when a text node carries a RESOLVED comment mark; returns that mark. */ +function resolvedCommentMarkOf(node: any): any | null { + if (!node || node.type !== "text" || !Array.isArray(node.marks)) return null; + return ( + node.marks.find( + (m: any) => + m && m.type === "comment" && m.attrs?.resolved === true && m.attrs?.commentId, + ) || null + ); +} + +/** + * Collect every RESOLVED inline-comment span in `doc`, in document order. Within + * each block's direct content, a maximal run of consecutive text nodes sharing + * the same resolved `commentId` is ONE span; its concatenated raw text is the + * selection used to re-anchor it elsewhere. Active (unresolved) comment marks are + * ignored — they survive a markdown round-trip on their own (a page read emits + * their `` wrapper), whereas resolved anchors are hidden + * from agent reads (#337) and would be erased by a full-body markdown rewrite. + */ +export function collectResolvedCommentSpans(doc: any): ResolvedCommentSpan[] { + const spans: ResolvedCommentSpan[] = []; + const visit = (node: any, depth: number): void => { + if (depth > MAX_DEPTH || !node || typeof node !== "object") return; + if (!Array.isArray(node.content)) return; + const content = node.content; + let i = 0; + while (i < content.length) { + const mark = resolvedCommentMarkOf(content[i]); + if (mark) { + const commentId = mark.attrs.commentId; + let text = ""; + let j = i; + while (j < content.length) { + const mj = resolvedCommentMarkOf(content[j]); + if (!mj || mj.attrs.commentId !== commentId) break; + text += typeof content[j].text === "string" ? content[j].text : ""; + j++; + } + if (text.length > 0) spans.push({ commentId, mark, text }); + i = j > i ? j : i + 1; + } else { + i++; + } + } + for (const child of content) { + if (child && typeof child === "object" && Array.isArray(child.content)) { + visit(child, depth + 1); + } + } + }; + visit(doc, 0); + return spans; +} + +/** + * Re-graft RESOLVED comment marks from `oldDoc` onto matching text ranges in + * `newDoc`, returning a NEW doc (never mutates the inputs). + * + * WHY (#493): an agent read hides resolved-comment anchors (#337), so the + * markdown it sends to a FULL-body rewrite (`updatePageMarkdown`) no longer + * carries them — a naive full write would erase every resolved comment mark. + * This restores them: each resolved span from the previous document is re-anchored + * onto the SAME text in the newly-imported body (first occurrence, using the + * shared anchoring / markdown-strip fallback), preserving `resolved:true` and the + * stored attrs. A span whose text the agent changed or deleted simply does not + * re-anchor and is dropped (its anchor is gone; it was already resolved). Active + * comments are untouched — they ride through the markdown themselves. + */ +export function regraftResolvedComments(oldDoc: any, newDoc: T): T { + if (!newDoc || typeof newDoc !== "object") return newDoc; + const spans = collectResolvedCommentSpans(oldDoc); + if (spans.length === 0) return newDoc; + const out = + typeof structuredClone === "function" + ? structuredClone(newDoc) + : (JSON.parse(JSON.stringify(newDoc)) as T); + for (const span of spans) { + // Clone the mark so the new document never shares a mark object with oldDoc. + const markClone = { type: "comment", attrs: { ...span.mark.attrs } }; + applyCommentMarkInDoc(out, span.text, markClone); + } + return out; +} diff --git a/packages/mcp/test/unit/regraft-resolved-comments.test.mjs b/packages/mcp/test/unit/regraft-resolved-comments.test.mjs new file mode 100644 index 00000000..fa3c2fa8 --- /dev/null +++ b/packages/mcp/test/unit/regraft-resolved-comments.test.mjs @@ -0,0 +1,107 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; + +import { + collectResolvedCommentSpans, + regraftResolvedComments, + applyCommentMarkInDoc, +} from "../../build/lib/comment-anchor.js"; + +/** + * #493 commit 6 — resolved-comment anchors must survive a full markdown rewrite + * (updatePageMarkdown). An agent read HIDES resolved anchors (#337), so its + * markdown drops them; a naive full write would erase the resolved comment marks. + * `regraftResolvedComments(oldDoc, newDoc)` re-anchors them onto the matching + * text. These exercise the real anchoring (no mock). + */ + +const doc = (...content) => ({ type: "doc", content }); +const para = (...content) => ({ type: "paragraph", content }); +const text = (t, marks) => (marks ? { type: "text", text: t, marks } : { type: "text", text: t }); +const resolvedComment = (commentId) => ({ type: "comment", attrs: { commentId, resolved: true } }); +const activeComment = (commentId) => ({ type: "comment", attrs: { commentId, resolved: false } }); + +/** The comment mark on a text node, or null. */ +function commentMarkOf(node) { + const marks = Array.isArray(node?.marks) ? node.marks : []; + return marks.find((m) => m && m.type === "comment") || null; +} +/** Flatten every text node in a doc (deep). */ +function textNodes(node, out = []) { + if (!node || typeof node !== "object") return out; + if (node.type === "text") out.push(node); + if (Array.isArray(node.content)) for (const c of node.content) textNodes(c, out); + return out; +} + +test("collectResolvedCommentSpans: only resolved marks, concatenated across a run", () => { + const old = doc( + para( + text("keep "), + text("resolved bit", [resolvedComment("r1")]), + text(" and "), + text("active bit", [activeComment("a1")]), + ), + ); + const spans = collectResolvedCommentSpans(old); + assert.equal(spans.length, 1); + assert.equal(spans[0].commentId, "r1"); + assert.equal(spans[0].text, "resolved bit"); + assert.equal(spans[0].mark.attrs.resolved, true); +}); + +test("regraft restores a resolved mark the agent's markdown dropped", () => { + // OLD doc has a resolved comment on "important note". + const old = doc(para(text("An "), text("important note", [resolvedComment("r1")]), text(" here."))); + // NEW doc (re-imported from the agent's markdown) has the SAME text but NO + // comment mark — the resolved anchor was hidden on read. + const fresh = doc(para(text("An important note here."))); + + const out = regraftResolvedComments(old, fresh); + // Inputs are not mutated. + assert.equal(commentMarkOf(textNodes(fresh)[0]), null); + // The resolved mark is back on exactly "important note". + const marked = textNodes(out).filter((n) => commentMarkOf(n)); + assert.equal(marked.length, 1); + assert.equal(marked[0].text, "important note"); + assert.equal(commentMarkOf(marked[0]).attrs.commentId, "r1"); + assert.equal(commentMarkOf(marked[0]).attrs.resolved, true); +}); + +test("a resolved span whose text the agent changed is dropped (no re-anchor)", () => { + const old = doc(para(text("stale text", [resolvedComment("r1")]))); + const fresh = doc(para(text("completely rewritten body"))); + const out = regraftResolvedComments(old, fresh); + assert.equal(textNodes(out).filter((n) => commentMarkOf(n)).length, 0); +}); + +test("regraft is a no-op when the old doc has no resolved comments", () => { + const old = doc(para(text("plain "), text("active", [activeComment("a1")]))); + const fresh = doc(para(text("plain active"))); + const out = regraftResolvedComments(old, fresh); + assert.equal(textNodes(out).filter((n) => commentMarkOf(n)).length, 0); +}); + +test("multiple distinct resolved comments are all restored", () => { + const old = doc( + para(text("first", [resolvedComment("r1")]), text(" middle "), text("second", [resolvedComment("r2")])), + ); + const fresh = doc(para(text("first middle second"))); + const out = regraftResolvedComments(old, fresh); + const byId = Object.fromEntries( + textNodes(out) + .filter((n) => commentMarkOf(n)) + .map((n) => [commentMarkOf(n).attrs.commentId, n.text]), + ); + assert.equal(byId["r1"], "first"); + assert.equal(byId["r2"], "second"); +}); + +test("applyCommentMarkInDoc preserves an arbitrary mark's attrs (resolved:true)", () => { + const d = doc(para(text("anchor me somewhere"))); + const ok = applyCommentMarkInDoc(d, "anchor me", { type: "comment", attrs: { commentId: "x9", resolved: true } }); + assert.equal(ok, true); + const marked = textNodes(d).filter((n) => commentMarkOf(n)); + assert.equal(marked[0].text, "anchor me"); + assert.equal(commentMarkOf(marked[0]).attrs.resolved, true); +});