From 443ad3a85680ed05c864fbf908fed90385381da4 Mon Sep 17 00:00:00 2001 From: claude code agent 227 Date: Thu, 25 Jun 2026 04:07:21 +0300 Subject: [PATCH] fix(mcp): replaceImage no longer yanks the cursor (#164) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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) --- packages/mcp/build/client.js | 19 +++--- packages/mcp/src/client.ts | 22 +++--- .../unit/comment-cursor-stability.test.mjs | 67 +++++++++++++++++++ 3 files changed, 85 insertions(+), 23 deletions(-) diff --git a/packages/mcp/build/client.js b/packages/mcp/build/client.js index 302d2a15..46380a0c 100644 --- a/packages/mcp/build/client.js +++ b/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))); diff --git a/packages/mcp/src/client.ts b/packages/mcp/src/client.ts index 5a8aaaf7..6293d5ee 100644 --- a/packages/mcp/src/client.ts +++ b/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; diff --git a/packages/mcp/test/unit/comment-cursor-stability.test.mjs b/packages/mcp/test/unit/comment-cursor-stability.test.mjs index 23614fb9..1bcca2af 100644 --- a/packages/mcp/test/unit/comment-cursor-stability.test.mjs +++ b/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"); +});