From fd92eff7d6030d571a02e095e93f2d9c2b6ad8a0 Mon Sep 17 00:00:00 2001 From: claude code agent 227 Date: Wed, 24 Jun 2026 05:56:05 +0300 Subject: [PATCH] fix(mcp): structural-diff write-back so agent edits don't jump the cursor (#152) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit mutatePageContent wrote agent edits back by DELETING the whole Yjs fragment and re-applying a fresh Y.Doc. Yjs is a CRDT — the editor anchors its selection to node ids — so wiping every id made an open editor's cursor lose its anchor and snap to the end of the document on every agent write. It was most visible on comment anchoring (issue #152): a comment changes no text, yet the cursor jumped. (Before commit 4201f0a3 the anchoring silently no-op'd, so the destructive write never ran for comments — hence the regression.) Fix: write via (y-prosemirror) — the same routine the editor uses to sync its own edits into Yjs. It structurally diffs the new doc against the live fragment and touches only changed nodes, preserving the ids of unchanged ones, so the cursor stays put. This improves ALL agent write tools (text edits, node ops, comments, replace) — minimal diff instead of full replace: less collab noise, stable block-ids, other users' cursors no longer disrupted. - collaboration.ts: new (sanitize -> PMNode.fromJSON against a memoized docmost schema -> updateYFragment in one transact), keeping the findUnstorableAttr encode diagnostic; swap the destructive write-back for it. - package.json: y-prosemirror promoted to a direct dependency (was transitive). - test: comment-cursor-stability.test.mjs — a Yjs RelativePosition (the cursor anchor) survives both a sibling edit and a comment-mark anchoring (the old full-replace tombstoned it -> null). 292 package tests green. Co-Authored-By: Claude Opus 4.8 --- packages/mcp/build/lib/collaboration.js | 55 +++++++++++--- packages/mcp/node_modules/y-prosemirror | 1 + packages/mcp/package.json | 1 + packages/mcp/src/lib/collaboration.ts | 58 +++++++++++--- .../unit/comment-cursor-stability.test.mjs | 75 +++++++++++++++++++ pnpm-lock.yaml | 3 + 6 files changed, 173 insertions(+), 20 deletions(-) create mode 120000 packages/mcp/node_modules/y-prosemirror create mode 100644 packages/mcp/test/unit/comment-cursor-stability.test.mjs 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)