diff --git a/packages/mcp/build/lib/collaboration.js b/packages/mcp/build/lib/collaboration.js index 5140acee..fc72bbf3 100644 --- a/packages/mcp/build/lib/collaboration.js +++ b/packages/mcp/build/lib/collaboration.js @@ -4,11 +4,25 @@ import * as Y from "yjs"; import WebSocket from "ws"; import { marked } from "marked"; import { generateJSON } from "@tiptap/html"; +import { Node as PMNode } from "@tiptap/pm/model"; +import { updateYFragment } from "y-prosemirror"; import { JSDOM } from "jsdom"; -import { docmostExtensions } from "./docmost-schema.js"; +import { docmostExtensions, docmostSchema } from "./docmost-schema.js"; import { withPageLock } from "./page-lock.js"; import { sanitizeForYjs, findUnstorableAttr } from "./node-ops.js"; import { summarizeChange } from "./diff.js"; +/** + * Build the descriptive error for an opaque Yjs encode failure ("Unexpected + * content type"), shared by both encode paths (`buildYDoc` -> `toYdoc` and + * `applyDocToFragment` -> `updateYFragment`) so the message wording stays in one + * place. `label` names the stage that failed (diagnostic). `sanitizeForYjs` + * already stripped `undefined` attrs, so a remaining failure is pinpointed via + * `findUnstorableAttr`. + */ +function unstorableYjsError(safe, label, e) { + const bad = findUnstorableAttr(safe); + return new Error(`Failed to encode document to Yjs (${label}): ${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)."}`); +} // Setup DOM environment for Tiptap HTML parsing in Node.js const dom = new JSDOM("
"); global.window = dom.window; @@ -446,17 +460,74 @@ export function buildYDoc(doc) { return TiptapTransformer.toYdoc(safe, "default", docmostExtensions); } catch (e) { - const bad = findUnstorableAttr(safe); - 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)."}`); + throw unstorableYjsError(safe, "toYdoc", e); } } /** - * 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 - * the dry-run preview so it fails identically to apply. + * 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"); + // Hydrate the ProseMirror node in its OWN try so a failure here (e.g. an + // unknown node type) is labelled "fromJSON" — the stage that actually threw — + // instead of being misattributed to the Yjs write stage (#154 review). + let pmNode; + try { + pmNode = PMNode.fromJSON(docmostSchema, safe); + } + catch (e) { + throw unstorableYjsError(safe, "fromJSON", e); + } + try { + ydoc.transact(() => { + updateYFragment(ydoc, fragment, pmNode, { + mapping: new Map(), + isOMark: new Map(), + }); + }); + } + catch (e) { + throw unstorableYjsError(safe, "updateYFragment", e); + } +} +/** + * Run an independent Yjs-encodability check (the same `sanitizeForYjs` + schema + * the apply path uses) and throw the same descriptive error when the doc cannot + * be stored. Used by the dry-run preview. + * + * Note: it does NOT run `updateYFragment` against the live fragment, so it is an + * encodability GATE, not a byte-for-byte rehearsal of apply — `buildYDoc` + * (`toYdoc`) and `applyDocToFragment` (`updateYFragment`) are two different + * encoders that nonetheless reject the same unstorable attributes. To narrow the + * preview/apply gap it ALSO rehearses the apply path's `PMNode.fromJSON` + * hydration, so a doc that would only fail there (e.g. an unknown node type) is + * rejected at preview time too (#154 review). Still cheap: no live fragment, no + * `updateYFragment`. */ export function assertYjsEncodable(doc) { buildYDoc(doc); + const safe = sanitizeForYjs(doc); + try { + PMNode.fromJSON(docmostSchema, safe); + } + catch (e) { + throw unstorableYjsError(safe, "fromJSON", e); + } } /** Time we wait for the initial handshake/sync before giving up. */ const CONNECT_TIMEOUT_MS = 25000; @@ -649,16 +720,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/build/lib/diff.js b/packages/mcp/build/lib/diff.js index 516a3c81..c19ff9a9 100644 --- a/packages/mcp/build/lib/diff.js +++ b/packages/mcp/build/lib/diff.js @@ -16,13 +16,10 @@ * If recreateTransform / the changeset throws on a pathological document pair, * we fall back to a coarse block-level text diff so the tool never hard-fails. */ -import { getSchema } from "@tiptap/core"; import { Node } from "@tiptap/pm/model"; import { ChangeSet, simplifyChanges } from "@tiptap/pm/changeset"; import { recreateTransform } from "@fellow/prosemirror-recreate-transform"; -import { docmostExtensions } from "./docmost-schema.js"; -/** Build the schema once; it is pure and reused across calls. */ -const schema = getSchema(docmostExtensions); +import { docmostSchema } from "./docmost-schema.js"; /** Recursively concatenate the plain text of a JSON node. */ function plainText(node) { if (!node || typeof node !== "object") @@ -242,8 +239,8 @@ export function diffDocs(oldDocJson, newDocJson, notesHeading = "Примеча let fellBack = false; const changedBlocks = new Set(); try { - const oldNode = Node.fromJSON(schema, oldDocJson); - const newNode = Node.fromJSON(schema, newDocJson); + const oldNode = Node.fromJSON(docmostSchema, oldDocJson); + const newNode = Node.fromJSON(docmostSchema, newDocJson); const tr = recreateTransform(oldNode, newNode, { complexSteps: false, wordDiffs: true, diff --git a/packages/mcp/build/lib/docmost-schema.js b/packages/mcp/build/lib/docmost-schema.js index 976e2d7f..6b6c221d 100644 --- a/packages/mcp/build/lib/docmost-schema.js +++ b/packages/mcp/build/lib/docmost-schema.js @@ -14,7 +14,7 @@ import TaskItem from "@tiptap/extension-task-item"; import Highlight from "@tiptap/extension-highlight"; import Subscript from "@tiptap/extension-subscript"; import Superscript from "@tiptap/extension-superscript"; -import { Node, Extension, Mark } from "@tiptap/core"; +import { Node, Extension, Mark, getSchema } from "@tiptap/core"; // Inlined from @tiptap/core's getStyleProperty (added after 3.20.x) so this // package can stay on the same @tiptap/core version as the editor and avoid a // duplicate-tiptap version split in the monorepo. Reads a single declaration @@ -1126,3 +1126,10 @@ export const docmostExtensions = [ PageBreak, DocmostAttributes, ]; +/** + * The ProseMirror schema for the docmost editor, built ONCE from + * `docmostExtensions`. Pure and reused by every consumer (diff, collaboration + * write-back) so the schema can never drift between call sites — it lives next + * to the extension list it is derived from. + */ +export const docmostSchema = getSchema(docmostExtensions); 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/client.ts b/packages/mcp/src/client.ts index 9873d119..bd891fc9 100644 --- a/packages/mcp/src/client.ts +++ b/packages/mcp/src/client.ts @@ -2995,9 +2995,9 @@ export class DocmostClient { const raw = await this.getPageRaw(pageId); const current = raw.content || { type: "doc", content: [] }; runTransform(current); - // Exercise the same Yjs encoder the apply path uses, so the preview - // fails with the SAME descriptive error when the doc is not encodable - // instead of returning a misleadingly-green diff. + // Run an independent Yjs-encodability check (same sanitize + schema as the + // apply path), so the preview fails with the same descriptive error when + // the doc is not encodable instead of returning a misleadingly-green diff. assertYjsEncodable(newDoc); return { pushed: false, diff --git a/packages/mcp/src/lib/collaboration.ts b/packages/mcp/src/lib/collaboration.ts index 6f0ad011..efc7bf17 100644 --- a/packages/mcp/src/lib/collaboration.ts +++ b/packages/mcp/src/lib/collaboration.ts @@ -4,12 +4,29 @@ import * as Y from "yjs"; import WebSocket from "ws"; import { marked } from "marked"; import { generateJSON } from "@tiptap/html"; +import { Node as PMNode } from "@tiptap/pm/model"; +import { updateYFragment } from "y-prosemirror"; import { JSDOM } from "jsdom"; -import { docmostExtensions } from "./docmost-schema.js"; +import { docmostExtensions, docmostSchema } from "./docmost-schema.js"; import { withPageLock } from "./page-lock.js"; import { sanitizeForYjs, findUnstorableAttr } from "./node-ops.js"; import { summarizeChange, VerifyReport } from "./diff.js"; +/** + * Build the descriptive error for an opaque Yjs encode failure ("Unexpected + * content type"), shared by both encode paths (`buildYDoc` -> `toYdoc` and + * `applyDocToFragment` -> `updateYFragment`) so the message wording stays in one + * place. `label` names the stage that failed (diagnostic). `sanitizeForYjs` + * already stripped `undefined` attrs, so a remaining failure is pinpointed via + * `findUnstorableAttr`. + */ +function unstorableYjsError(safe: any, label: string, e: unknown): Error { + const bad = findUnstorableAttr(safe); + return new Error( + `Failed to encode document to Yjs (${label}): ${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)."}`, + ); +} + /** * 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 @@ -499,20 +516,73 @@ export function buildYDoc(doc: any): Y.Doc { try { return TiptapTransformer.toYdoc(safe, "default", docmostExtensions); } catch (e) { - const bad = findUnstorableAttr(safe); - 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)."}`, - ); + throw unstorableYjsError(safe, "toYdoc", e); } } /** - * 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 - * the dry-run preview so it fails identically to apply. + * 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"); + // Hydrate the ProseMirror node in its OWN try so a failure here (e.g. an + // unknown node type) is labelled "fromJSON" — the stage that actually threw — + // instead of being misattributed to the Yjs write stage (#154 review). + let pmNode: PMNode; + try { + pmNode = PMNode.fromJSON(docmostSchema, safe); + } catch (e) { + throw unstorableYjsError(safe, "fromJSON", e); + } + try { + ydoc.transact(() => { + updateYFragment(ydoc, fragment, pmNode, { + mapping: new Map(), + isOMark: new Map(), + }); + }); + } catch (e) { + throw unstorableYjsError(safe, "updateYFragment", e); + } +} + +/** + * Run an independent Yjs-encodability check (the same `sanitizeForYjs` + schema + * the apply path uses) and throw the same descriptive error when the doc cannot + * be stored. Used by the dry-run preview. + * + * Note: it does NOT run `updateYFragment` against the live fragment, so it is an + * encodability GATE, not a byte-for-byte rehearsal of apply — `buildYDoc` + * (`toYdoc`) and `applyDocToFragment` (`updateYFragment`) are two different + * encoders that nonetheless reject the same unstorable attributes. To narrow the + * preview/apply gap it ALSO rehearses the apply path's `PMNode.fromJSON` + * hydration, so a doc that would only fail there (e.g. an unknown node type) is + * rejected at preview time too (#154 review). Still cheap: no live fragment, no + * `updateYFragment`. */ export function assertYjsEncodable(doc: any): void { buildYDoc(doc); + const safe = sanitizeForYjs(doc); + try { + PMNode.fromJSON(docmostSchema, safe); + } catch (e) { + throw unstorableYjsError(safe, "fromJSON", e); + } } /** Time we wait for the initial handshake/sync before giving up. */ @@ -727,16 +797,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/src/lib/diff.ts b/packages/mcp/src/lib/diff.ts index d0848997..ba216df4 100644 --- a/packages/mcp/src/lib/diff.ts +++ b/packages/mcp/src/lib/diff.ts @@ -17,11 +17,10 @@ * we fall back to a coarse block-level text diff so the tool never hard-fails. */ -import { getSchema } from "@tiptap/core"; import { Node } from "@tiptap/pm/model"; import { ChangeSet, simplifyChanges } from "@tiptap/pm/changeset"; import { recreateTransform } from "@fellow/prosemirror-recreate-transform"; -import { docmostExtensions } from "./docmost-schema.js"; +import { docmostSchema } from "./docmost-schema.js"; /** A single inserted/deleted change with its containing-block context. */ export interface DiffChange { @@ -49,8 +48,6 @@ export interface DiffResult { markdown: string; } -/** Build the schema once; it is pure and reused across calls. */ -const schema = getSchema(docmostExtensions); /** Recursively concatenate the plain text of a JSON node. */ function plainText(node: any): string { @@ -288,8 +285,8 @@ export function diffDocs( const changedBlocks = new Set