From 6bb9dfdc86c070b8ce182c5bd2b675884ca05df4 Mon Sep 17 00:00:00 2001 From: claude code agent 227 Date: Fri, 26 Jun 2026 06:09:02 +0300 Subject: [PATCH] fix(editor-ext): dedupe colliding unique ids on import/normalize (#206) editor-pm-7: addUniqueIdsToDoc only FILLED missing ids and never deduplicated existing ones, so a copy/paste or bulk-JSON duplicate that kept its attrs.id produced two nodes sharing an id. MCP addressed edits (patch_node / delete_node "before/after id") then hit the wrong node or both. Walk the configured-type nodes in document order: the first occurrence of an id keeps it (stable anchor), later duplicates are reassigned a fresh id. transclusionSource ids are cross-reference keys (references resolve a source by this id), so they are only filled-when-missing, never reassigned, to avoid orphaning their references. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/lib/unique-id/unique-id.util.test.ts | 55 +++++++++++++++++++ .../src/lib/unique-id/unique-id.util.ts | 38 +++++++++++-- 2 files changed, 87 insertions(+), 6 deletions(-) create mode 100644 packages/editor-ext/src/lib/unique-id/unique-id.util.test.ts diff --git a/packages/editor-ext/src/lib/unique-id/unique-id.util.test.ts b/packages/editor-ext/src/lib/unique-id/unique-id.util.test.ts new file mode 100644 index 00000000..eddc1dc9 --- /dev/null +++ b/packages/editor-ext/src/lib/unique-id/unique-id.util.test.ts @@ -0,0 +1,55 @@ +import { describe, it, expect } from "vitest"; +import StarterKit from "@tiptap/starter-kit"; +import { addUniqueIdsToDoc } from "./unique-id.util"; +import { UniqueID } from "./unique-id"; + +// Minimal extension set: StarterKit (paragraph/heading) + the UniqueID config +// the server uses for the addressing anchors. +const extensions = [ + StarterKit, + UniqueID.configure({ types: ["heading", "paragraph"] }), +]; + +const para = (id: string | undefined, text: string) => ({ + type: "paragraph", + ...(id !== undefined ? { attrs: { id } } : {}), + content: [{ type: "text", text }], +}); + +const ids = (doc: any): (string | undefined)[] => + (doc.content ?? []).map((n: any) => n.attrs?.id); + +describe("addUniqueIdsToDoc", () => { + it("fills ids on nodes that are missing one", () => { + const doc = { type: "doc", content: [para(undefined, "a"), para(undefined, "b")] }; + const out = addUniqueIdsToDoc(doc, extensions); + const [a, b] = ids(out); + expect(a).toBeTruthy(); + expect(b).toBeTruthy(); + expect(a).not.toBe(b); + }); + + it("deduplicates two nodes that share the same id (#206 editor-pm-7)", () => { + // A copy/paste or bulk-JSON duplicate keeps the original id on both nodes. + const doc = { + type: "doc", + content: [para("dup", "first"), para("dup", "second")], + }; + const out = addUniqueIdsToDoc(doc, extensions); + const [first, second] = ids(out); + // The first occurrence keeps the id (stable anchor); the duplicate is + // reassigned a fresh one so MCP addressing can't hit the wrong/both nodes. + expect(first).toBe("dup"); + expect(second).toBeTruthy(); + expect(second).not.toBe("dup"); + }); + + it("leaves already-unique ids untouched", () => { + const doc = { + type: "doc", + content: [para("x1", "first"), para("x2", "second")], + }; + const out = addUniqueIdsToDoc(doc, extensions); + expect(ids(out)).toEqual(["x1", "x2"]); + }); +}); diff --git a/packages/editor-ext/src/lib/unique-id/unique-id.util.ts b/packages/editor-ext/src/lib/unique-id/unique-id.util.ts index 8d1991ed..88e81324 100644 --- a/packages/editor-ext/src/lib/unique-id/unique-id.util.ts +++ b/packages/editor-ext/src/lib/unique-id/unique-id.util.ts @@ -59,18 +59,44 @@ export function addUniqueIdsToDoc( ]); const contentNode = Node.fromJSON(schema, doc); - // Find nodes that don't have a unique ID - const nodesWithoutId = findChildren(contentNode, (node) => { - return !node.attrs[attributeName] && types.includes(node.type.name); + // All nodes of the configured types, in document order, so that the FIRST + // occurrence of any given id keeps it and later duplicates get reassigned. + const idNodes = findChildren(contentNode, (node) => { + return types.includes(node.type.name); }); - // Edit the document to add unique IDs to the nodes that don't have a unique ID + // `transclusionSource` ids are cross-reference keys (a transclusionReference / + // the page_transclusions table resolves a source by this id), so rewriting one + // would orphan its references. We only fill a MISSING id for those, never + // reassign an existing one; plain block anchors (heading/paragraph) are safe to + // dedupe. + const NO_REASSIGN = new Set(["transclusionSource"]); + + // Edit the document to (a) add ids where missing and (b) dedupe collisions. A + // duplicate id otherwise lets copy/paste/import produce two nodes sharing an + // id, so MCP addressed edits (patch_node / delete_node "before/after id") hit + // the wrong node or both (#206 editor-pm-7). This previously only filled + // missing ids and never deduplicated existing ones. + const seenIds = new Set(); let tr = EditorState.create({ doc: contentNode, }).tr; // eslint-disable-next-line no-restricted-syntax - for (const { node, pos } of nodesWithoutId) { - tr = tr.setNodeAttribute(pos, attributeName, generateID({ node, pos })); + for (const { node, pos } of idNodes) { + const currentId = node.attrs[attributeName]; + const isDuplicate = currentId != null && seenIds.has(currentId); + const needsNewId = + currentId == null || (isDuplicate && !NO_REASSIGN.has(node.type.name)); + + if (needsNewId) { + // setNodeAttribute only changes attributes (no size change), so positions + // from the original node stay valid across the whole loop. + const newId = generateID({ node, pos }); + tr = tr.setNodeAttribute(pos, attributeName, newId); + seenIds.add(newId); + } else if (currentId != null) { + seenIds.add(currentId); + } } // Return the updated document