diff --git a/packages/mcp/build/lib/collaboration.js b/packages/mcp/build/lib/collaboration.js index 5140acee..35bd7a13 100644 --- a/packages/mcp/build/lib/collaboration.js +++ b/packages/mcp/build/lib/collaboration.js @@ -4,11 +4,18 @@ import * as Y from "yjs"; import WebSocket from "ws"; import { marked } from "marked"; import { generateJSON } from "@tiptap/html"; +import { getSchema } from "@tiptap/core"; +import { Node as PMNode } from "@tiptap/pm/model"; +import { updateYFragment } from "y-prosemirror"; import { JSDOM } from "jsdom"; import { docmostExtensions } from "./docmost-schema.js"; import { withPageLock } from "./page-lock.js"; import { sanitizeForYjs, findUnstorableAttr } from "./node-ops.js"; import { summarizeChange } from "./diff.js"; +// The ProseMirror schema for the docmost editor, built once (mirrors diff.ts). +// `updateYFragment` needs a real PM Node, so we re-hydrate the transformed JSON +// against this schema before diffing it into the live Yjs fragment. +const docmostSchema = getSchema(docmostExtensions); // Setup DOM environment for Tiptap HTML parsing in Node.js const dom = new JSDOM(""); global.window = dom.window; @@ -450,6 +457,40 @@ export function buildYDoc(doc) { throw new Error(`Failed to encode document to Yjs (toYdoc): ${e instanceof Error ? e.message : String(e)}.${bad ? ` Offending attribute: ${bad}.` : " A node/mark attribute likely holds a value Yjs cannot store (e.g. undefined)."}`); } } +/** + * Write a new ProseMirror doc into the live Yjs fragment by STRUCTURAL DIFF, + * preserving the Yjs identity of unchanged nodes (issue #152). + * + * The previous approach deleted the whole fragment and re-applied a fresh Y.Doc, + * which discarded every Yjs node id. y-prosemirror anchors the editor selection + * to those ids, so an open editor's cursor lost its anchor and snapped to the + * end of the document on every agent write (most visibly on comment anchoring, + * which changes no text at all). `updateYFragment` is exactly the routine the + * editor itself uses to sync ProseMirror edits into Yjs: it diffs the new node + * against the current fragment and touches only the changed children, so + * unchanged nodes keep their ids and the live cursor stays put. + * + * Must run inside a single `transact` so the diff applies atomically (no remote + * update interleaves). Keeps `buildYDoc`'s `findUnstorableAttr` diagnostic for + * the opaque "Unexpected content type" encode failure. + */ +export function applyDocToFragment(ydoc, newDoc) { + const safe = sanitizeForYjs(newDoc); + const fragment = ydoc.getXmlFragment("default"); + try { + const pmNode = PMNode.fromJSON(docmostSchema, safe); + ydoc.transact(() => { + updateYFragment(ydoc, fragment, pmNode, { + mapping: new Map(), + isOMark: new Map(), + }); + }); + } + catch (e) { + const bad = findUnstorableAttr(safe); + throw new Error(`Failed to encode document to Yjs (updateYFragment): ${e instanceof Error ? e.message : String(e)}.${bad ? ` Offending attribute: ${bad}.` : " A node/mark attribute likely holds a value Yjs cannot store (e.g. undefined)."}`); + } +} /** * Validate that a doc is Yjs-encodable by building (and discarding) a Y.Doc. * Throws the same descriptive error as the apply path when it is not. Used by @@ -649,16 +690,10 @@ export async function mutatePageContent(pageId, collabToken, baseUrl, transform) finish(null, mutationResult); return; } - const tempDoc = buildYDoc(newDoc); - // Fetch the fragment immediately before the transact that mutates - // it, rather than reusing a handle grabbed across the transform. - const fragment = ydoc.getXmlFragment("default"); - ydoc.transact(() => { - if (fragment.length > 0) { - fragment.delete(0, fragment.length); - } - Y.applyUpdate(ydoc, Y.encodeStateAsUpdate(tempDoc)); - }); + // Structural diff into the live fragment (issue #152): preserves + // the Yjs ids of unchanged nodes, so an open editor's cursor is not + // yanked to the end of the document on every agent write. + applyDocToFragment(ydoc, newDoc); } catch (e) { // Includes errors thrown by transform (e.g. "afterText not found", diff --git a/packages/mcp/node_modules/y-prosemirror b/packages/mcp/node_modules/y-prosemirror new file mode 120000 index 00000000..16997d1b --- /dev/null +++ b/packages/mcp/node_modules/y-prosemirror @@ -0,0 +1 @@ +../../../node_modules/.pnpm/y-prosemirror@1.3.7_prosemirror-model@1.25.1_prosemirror-state@1.4.3_prosemirror-view@1_0ad6648b7e1f6d6f3287a40e0e62139b/node_modules/y-prosemirror \ No newline at end of file diff --git a/packages/mcp/package.json b/packages/mcp/package.json index 2b1074fb..3edc1902 100644 --- a/packages/mcp/package.json +++ b/packages/mcp/package.json @@ -52,6 +52,7 @@ "jsdom": "^27.4.0", "marked": "^17.0.1", "ws": "^8.19.0", + "y-prosemirror": "1.3.7", "yjs": "^13.6.29", "zod": "^3.22.0" }, diff --git a/packages/mcp/src/lib/collaboration.ts b/packages/mcp/src/lib/collaboration.ts index 6f0ad011..cb84f410 100644 --- a/packages/mcp/src/lib/collaboration.ts +++ b/packages/mcp/src/lib/collaboration.ts @@ -4,12 +4,20 @@ import * as Y from "yjs"; import WebSocket from "ws"; import { marked } from "marked"; import { generateJSON } from "@tiptap/html"; +import { getSchema } from "@tiptap/core"; +import { Node as PMNode } from "@tiptap/pm/model"; +import { updateYFragment } from "y-prosemirror"; import { JSDOM } from "jsdom"; import { docmostExtensions } from "./docmost-schema.js"; import { withPageLock } from "./page-lock.js"; import { sanitizeForYjs, findUnstorableAttr } from "./node-ops.js"; import { summarizeChange, VerifyReport } from "./diff.js"; +// The ProseMirror schema for the docmost editor, built once (mirrors diff.ts). +// `updateYFragment` needs a real PM Node, so we re-hydrate the transformed JSON +// against this schema before diffing it into the live Yjs fragment. +const docmostSchema = getSchema(docmostExtensions); + /** * The resolved value of every content-mutating collab write: the document that * was written (or the live doc when the transform aborted) plus a verifiable @@ -506,6 +514,42 @@ export function buildYDoc(doc: any): Y.Doc { } } +/** + * Write a new ProseMirror doc into the live Yjs fragment by STRUCTURAL DIFF, + * preserving the Yjs identity of unchanged nodes (issue #152). + * + * The previous approach deleted the whole fragment and re-applied a fresh Y.Doc, + * which discarded every Yjs node id. y-prosemirror anchors the editor selection + * to those ids, so an open editor's cursor lost its anchor and snapped to the + * end of the document on every agent write (most visibly on comment anchoring, + * which changes no text at all). `updateYFragment` is exactly the routine the + * editor itself uses to sync ProseMirror edits into Yjs: it diffs the new node + * against the current fragment and touches only the changed children, so + * unchanged nodes keep their ids and the live cursor stays put. + * + * Must run inside a single `transact` so the diff applies atomically (no remote + * update interleaves). Keeps `buildYDoc`'s `findUnstorableAttr` diagnostic for + * the opaque "Unexpected content type" encode failure. + */ +export function applyDocToFragment(ydoc: Y.Doc, newDoc: any): void { + const safe = sanitizeForYjs(newDoc); + const fragment = ydoc.getXmlFragment("default"); + try { + const pmNode = PMNode.fromJSON(docmostSchema, safe); + ydoc.transact(() => { + updateYFragment(ydoc, fragment, pmNode, { + mapping: new Map(), + isOMark: new Map(), + }); + }); + } catch (e) { + const bad = findUnstorableAttr(safe); + throw new Error( + `Failed to encode document to Yjs (updateYFragment): ${e instanceof Error ? e.message : String(e)}.${bad ? ` Offending attribute: ${bad}.` : " A node/mark attribute likely holds a value Yjs cannot store (e.g. undefined)."}`, + ); + } +} + /** * Validate that a doc is Yjs-encodable by building (and discarding) a Y.Doc. * Throws the same descriptive error as the apply path when it is not. Used by @@ -727,16 +771,10 @@ export async function mutatePageContent( return; } - const tempDoc = buildYDoc(newDoc); - // Fetch the fragment immediately before the transact that mutates - // it, rather than reusing a handle grabbed across the transform. - const fragment = ydoc.getXmlFragment("default"); - ydoc.transact(() => { - if (fragment.length > 0) { - fragment.delete(0, fragment.length); - } - Y.applyUpdate(ydoc, Y.encodeStateAsUpdate(tempDoc)); - }); + // Structural diff into the live fragment (issue #152): preserves + // the Yjs ids of unchanged nodes, so an open editor's cursor is not + // yanked to the end of the document on every agent write. + applyDocToFragment(ydoc, newDoc); } catch (e) { // Includes errors thrown by transform (e.g. "afterText not found", // "text not found"): propagate them verbatim to the caller. diff --git a/packages/mcp/test/unit/comment-cursor-stability.test.mjs b/packages/mcp/test/unit/comment-cursor-stability.test.mjs new file mode 100644 index 00000000..e494d131 --- /dev/null +++ b/packages/mcp/test/unit/comment-cursor-stability.test.mjs @@ -0,0 +1,75 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import * as Y from "yjs"; +import { applyDocToFragment } from "../../build/lib/collaboration.js"; + +// Regression for issue #152: agent writes (comment anchoring especially) must +// NOT yank the open editor's cursor to the end of the document. The cursor is a +// Yjs RelativePosition anchored to node ids; the old write-back deleted the whole +// fragment and rebuilt it, destroying every id, so the position no longer +// resolved. `applyDocToFragment` uses `updateYFragment` (the editor's own diff), +// which keeps unchanged nodes' ids — so a RelativePosition still resolves. + +const para = (text, marks) => ({ + type: "paragraph", + content: [{ type: "text", text, ...(marks ? { marks } : {}) }], +}); +const doc = (...paras) => ({ type: "doc", content: paras }); + +/** The XmlText of the Nth paragraph in the live fragment. */ +function paragraphText(ydoc, n) { + const el = ydoc.getXmlFragment("default").get(n); // XmlElement + return el.get(0); // its XmlText child +} + +test("an UNCHANGED node keeps its Yjs identity across an edit (cursor survives)", () => { + const ydoc = new Y.Doc(); + applyDocToFragment(ydoc, doc(para("Hello world"), para("Second"))); + + // Anchor a cursor at offset 5 inside the FIRST (soon-to-be-unchanged) paragraph. + const relPos = Y.createRelativePositionFromTypeIndex(paragraphText(ydoc, 0), 5); + + // Edit only the SECOND paragraph; the first is untouched. + applyDocToFragment(ydoc, doc(para("Hello world"), para("Second edited"))); + + const abs = Y.createAbsolutePositionFromRelativePosition(relPos, ydoc); + assert.notEqual(abs, null, "the cursor's relative position must still resolve"); + assert.equal(abs.index, 5, "the cursor must stay at the same offset"); + // And the edit actually landed. + assert.equal(paragraphText(ydoc, 1).toString(), "Second edited"); +}); + +test("anchoring a comment mark keeps the cursor in the marked text (issue #152)", () => { + const ydoc = new Y.Doc(); + applyDocToFragment(ydoc, doc(para("Hello world"))); + + // The user's cursor sits inside the text that is about to be commented. + const relPos = Y.createRelativePositionFromTypeIndex(paragraphText(ydoc, 0), 3); + + // Agent anchors a comment over "Hello" — text is identical, only a mark added. + applyDocToFragment( + ydoc, + doc({ + type: "paragraph", + content: [ + { + type: "text", + text: "Hello", + marks: [ + { type: "comment", attrs: { commentId: "c1", resolved: false } }, + ], + }, + { type: "text", text: " world" }, + ], + }), + ); + + // The text is intact (the mark splits "Hello" / " world" but reads the same). + const para0 = ydoc.getXmlFragment("default").get(0); + assert.equal(para0.toString().replace(/<[^>]*>/g, ""), "Hello world"); + + // ...and the cursor anchored before the write still resolves (did not jump to + // the document end as it did with the destructive full-replace). + const abs = Y.createAbsolutePositionFromRelativePosition(relPos, ydoc); + assert.notEqual(abs, null, "comment anchoring must not destroy the cursor anchor"); +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index d6af709c..4a55e7a0 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -946,6 +946,9 @@ importers: ws: specifier: 8.20.1 version: 8.20.1 + y-prosemirror: + specifier: 1.3.7 + version: 1.3.7(prosemirror-model@1.25.1)(prosemirror-state@1.4.3)(prosemirror-view@1.40.0)(y-protocols@1.0.6(yjs@13.6.30(patch_hash=1ceeb66dba1f86545c98a3ff7f5152aff9b35caf409091cef9caedb5e65c8810)))(yjs@13.6.30(patch_hash=1ceeb66dba1f86545c98a3ff7f5152aff9b35caf409091cef9caedb5e65c8810)) yjs: specifier: ^13.6.29 version: 13.6.30(patch_hash=1ceeb66dba1f86545c98a3ff7f5152aff9b35caf409091cef9caedb5e65c8810)