fix(mcp): replaceImage no longer yanks the cursor (#164)

`mutateLiveContentUnlocked` — the write path used by `replaceImage` — still
did the pre-#152 destructive write (delete the whole fragment + applyUpdate a
fresh Y.Doc), discarding every Yjs node id. y-prosemirror anchors the editor
selection to those ids, so an open editor's cursor snapped to the document
end on every image swap, exactly the #152 jump that the main write path no
longer causes.

Switch it to the same `applyDocToFragment(ydoc, newDoc)` structural diff
(updateYFragment) as the main path, so unchanged nodes keep their ids and the
live cursor stays put. It runs its own atomic transact, so the old explicit
transact/delete is gone; the now-unused docmostExtensions import is dropped.

Regression tests (cursor-stability suite): a sibling paragraph's
RelativePosition survives a top-level image src/attachmentId swap, and an
image nested in a callout, matching the shapes replaceImage produces.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

packages/mcp/build/client.js

@@ -7,8 +7,7 @@ import { TiptapTransformer } from "@hocuspocus/transformer";
 import * as Y from "yjs";
 import WebSocket from "ws";
 import { convertProseMirrorToMarkdown } from "./lib/markdown-converter.js";
-import { updatePageContentRealtime, replacePageContent, markdownToProseMirror, mutatePageContent, buildCollabWsUrl, assertYjsEncodable, } from "./lib/collaboration.js";
-import { docmostExtensions } from "./lib/docmost-schema.js";
+import { updatePageContentRealtime, replacePageContent, markdownToProseMirror, mutatePageContent, buildCollabWsUrl, assertYjsEncodable, applyDocToFragment, } from "./lib/collaboration.js";
 import { footnoteWarningsField } from "./lib/footnote-analyze.js";
 import { buildPageTree } from "./lib/tree.js";
 import { serializeDocmostMarkdown, parseDocmostMarkdown, } from "./lib/markdown-document.js";
@@ -361,14 +360,14 @@ export class DocmostClient {
                             finish(null, mutationResult);
                             return;
                         }
-                        const tempDoc = TiptapTransformer.toYdoc(newDoc, "default", docmostExtensions);
-                        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), mirroring
+                        // the main write path: preserves the Yjs ids of unchanged nodes so
+                        // an open editor's cursor is not yanked to the end of the document.
+                        // The previous destructive rewrite (delete-all + applyUpdate of a
+                        // fresh Y.Doc) discarded every node id, so replaceImage — the only
+                        // caller of this method — still reproduced the #152 cursor jump
+                        // (#164). applyDocToFragment runs its own atomic `transact`.
+                        applyDocToFragment(ydoc, newDoc);
                     }
                     catch (e) {
                         finish(e instanceof Error ? e : new Error(String(e)));

packages/mcp/src/client.ts

@@ -20,9 +20,9 @@ import {
   mutatePageContent,
   buildCollabWsUrl,
   assertYjsEncodable,
+  applyDocToFragment,
   MutationResult,
 } from "./lib/collaboration.js";
-import { docmostExtensions } from "./lib/docmost-schema.js";
 import { footnoteWarningsField } from "./lib/footnote-analyze.js";
 import { buildPageTree } from "./lib/tree.js";
 import {
@@ -479,18 +479,14 @@ export class DocmostClient {
               return;
             }
 
-            const tempDoc = TiptapTransformer.toYdoc(
-              newDoc,
-              "default",
-              docmostExtensions,
-            );
-            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), mirroring
+            // the main write path: preserves the Yjs ids of unchanged nodes so
+            // an open editor's cursor is not yanked to the end of the document.
+            // The previous destructive rewrite (delete-all + applyUpdate of a
+            // fresh Y.Doc) discarded every node id, so replaceImage — the only
+            // caller of this method — still reproduced the #152 cursor jump
+            // (#164). applyDocToFragment runs its own atomic `transact`.
+            applyDocToFragment(ydoc, newDoc);
           } catch (e) {
             finish(e instanceof Error ? e : new Error(String(e)));
             return;

packages/mcp/test/unit/comment-cursor-stability.test.mjs

@@ -162,3 +162,70 @@ test("assertYjsEncodable rejects an un-hydratable doc at preview time (fromJSON
     /Failed to encode document to Yjs/,
   );
 });
+
+// Issue #164: `replaceImage` went through `mutateLiveContentUnlocked`, which
+// (unlike the main write path fixed in #152) still deleted the whole fragment
+// and re-applied a fresh Y.Doc — discarding every node id, so an open editor's
+// cursor jumped to the document end on an image swap. That method now uses the
+// same `applyDocToFragment`, so a sibling paragraph's cursor anchor survives an
+// image `src`/`attachmentId` replacement. These exercise that routine on the
+// image shapes `replaceImage` produces (top-level and nested in a callout).
+
+const image = (attachmentId, src) => ({
+  type: "image",
+  attrs: { attachmentId, src, width: "640", align: "center" },
+});
+
+test("replacing a top-level image keeps a sibling paragraph's cursor anchor (#164)", () => {
+  const ydoc = new Y.Doc();
+  applyDocToFragment(
+    ydoc,
+    doc(para("Caption above"), image("att-old", "/files/old.png")),
+  );
+
+  // The user's cursor sits in the (unchanged) caption paragraph.
+  const relPos = Y.createRelativePositionFromTypeIndex(paragraphText(ydoc, 0), 7);
+
+  // Agent repoints the image to a freshly uploaded attachment (new id + src).
+  applyDocToFragment(
+    ydoc,
+    doc(para("Caption above"), image("att-new", "/files/new.png")),
+  );
+
+  const abs = Y.createAbsolutePositionFromRelativePosition(relPos, ydoc);
+  assert.notEqual(abs, null, "the caption cursor anchor must still resolve");
+  assert.equal(abs.index, 7, "the cursor must stay at the same offset");
+  // The swap actually landed: the image now carries the new attachment id/src.
+  const img = ydoc.getXmlFragment("default").get(1);
+  assert.equal(img.nodeName, "image");
+  assert.equal(img.getAttribute("attachmentId"), "att-new");
+  assert.equal(img.getAttribute("src"), "/files/new.png");
+});
+
+test("replacing an image nested in a callout keeps an outer paragraph's anchor (#164)", () => {
+  const callout = (attachmentId, src) => ({
+    type: "callout",
+    attrs: { type: "info" },
+    content: [image(attachmentId, src)],
+  });
+  const ydoc = new Y.Doc();
+  applyDocToFragment(
+    ydoc,
+    doc(para("Intro paragraph"), callout("att-old", "/files/old.png")),
+  );
+
+  const relPos = Y.createRelativePositionFromTypeIndex(paragraphText(ydoc, 0), 5);
+
+  applyDocToFragment(
+    ydoc,
+    doc(para("Intro paragraph"), callout("att-new", "/files/new.png")),
+  );
+
+  const abs = Y.createAbsolutePositionFromRelativePosition(relPos, ydoc);
+  assert.notEqual(abs, null, "the outer paragraph anchor must still resolve");
+  assert.equal(abs.index, 5, "the cursor must stay at the same offset");
+  // The nested image was repointed.
+  const calloutEl = ydoc.getXmlFragment("default").get(1);
+  const img = calloutEl.get(0);
+  assert.equal(img.getAttribute("attachmentId"), "att-new");
+});