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) <noreply@anthropic.com>
This commit is contained in:
55
packages/editor-ext/src/lib/unique-id/unique-id.util.test.ts
Normal file
55
packages/editor-ext/src/lib/unique-id/unique-id.util.test.ts
Normal file
@@ -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"]);
|
||||
});
|
||||
});
|
||||
@@ -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<string>();
|
||||
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
|
||||
|
||||
Reference in New Issue
Block a user