From a3daccd1225008713d6b1fde0ce85d0574a02bbb Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Fri, 30 Oct 2026 17:41:57 -0700 Subject: [PATCH 1/6] [lexical-y] Chore: Add exact restored-extent cases for undoing a value's removal, with the ones main fails as expected failures ## Description Tests only, for #9623, #9620 and the D64 shape (#9560), so a fix for them is judged by exact extents instead of "on the word or nowhere": a looser oracle let a mutation that drops every restored value pass. Every case checks that all clients and a fresh read agree, and the exact characters the value is on. - `regressions/UndoRestoredRunEndExtents.test.ts` (244 cases): a collaborator removes a value (comment, bold, italic, underline, color, NodeState, link) from a word; its author undoes the value, maybe writing the same key on a later range before or after; a third client may delete text around the value's end; the collaborator undoes the removal: the value is back on exactly the word. Also 1 to 256 comment ids in one write, and the concurrent shape of #9620. Main fails every garbage-collection-on cell and the concurrent cells (`test.fails`). - `regressions/UndoRestoredRunEnds.test.ts` (151 cases): comment, bold, color and NodeState, removed from the word, part of it or the whole document, the author undoing first or not, text typed after the word, garbage collection on and off, two id orders; undo, redo and undo again, each with its exact extent. Main fails the author-undoes cells with garbage collection on (#9623) and the partial removals (the author's undo leaves the rest of the word with the value the removal restated, then the removal's undo gives the whole word back: D64), and #9620's two cells. - `UndoSpreadFuzz.test.ts`: two bound editors synced after every op, random comment adds, removals, formats, typing, deletes and undos; a comment id on two blocks fails. 60 seeds by default (`US_SEEDS`, `US_START`, `US_OPS`, `US_UNDO`); the 9 seeds main fails are `test.fails`. - `regressions/UndoCommentRemovalSpread.test.ts`: #9623 with a second comment and typing in the same undo step (4 cases, all fail on main). - `regressions/UndoKeepsLaterFormat.test.ts`: an undo of a delete keeps the italic a collaborator gave the text afterwards (passes on main; a fix that ends the runs an undo brings back where they ended at the delete broke it). ## Test plan main (1853707e) with these files: ``` $ vitest --run --project unit Test Files 5 passed (5) Tests 262 passed | 194 expected fail (456) ``` `pnpm run ci-check` passes. --- .../src/__tests__/unit/UndoSpreadFuzz.test.ts | 256 ++++++++++ .../UndoCommentRemovalSpread.test.ts | 173 +++++++ .../regressions/UndoKeepsLaterFormat.test.ts | 94 ++++ .../UndoRestoredRunEndExtents.test.ts | 311 +++++++++++++ .../regressions/UndoRestoredRunEnds.test.ts | 440 ++++++++++++++++++ 5 files changed, 1274 insertions(+) create mode 100644 packages/lexical-y/src/__tests__/unit/UndoSpreadFuzz.test.ts create mode 100644 packages/lexical-y/src/__tests__/unit/regressions/UndoCommentRemovalSpread.test.ts create mode 100644 packages/lexical-y/src/__tests__/unit/regressions/UndoKeepsLaterFormat.test.ts create mode 100644 packages/lexical-y/src/__tests__/unit/regressions/UndoRestoredRunEndExtents.test.ts create mode 100644 packages/lexical-y/src/__tests__/unit/regressions/UndoRestoredRunEnds.test.ts diff --git a/packages/lexical-y/src/__tests__/unit/UndoSpreadFuzz.test.ts b/packages/lexical-y/src/__tests__/unit/UndoSpreadFuzz.test.ts new file mode 100644 index 00000000000..510c58a7599 --- /dev/null +++ b/packages/lexical-y/src/__tests__/unit/UndoSpreadFuzz.test.ts @@ -0,0 +1,256 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + */ + +// @vitest-environment node + +// Fuzz: does an undo spread a comment onto other blocks? Two bound editors, +// synced after every op, run random comment adds (each inside one text +// node), comment removals ($removeMarkID over the whole document), formats, +// plain typing, deletes and undos. A comment id on two blocks is a spread +// (#9623: "X adds, Y removes, X undoes, Y undoes" put it on everything +// after it). Env: US_SEEDS (default 60), US_START, US_OPS (default 30), +// US_UNDO (default 0.15). + +import {buildEditorFromExtensions} from '@lexical/extension'; +import { + $addMarkID, + $getNodeMarkIDs, + $removeMarkID, + MarkStateExtension, +} from '@lexical/mark'; +import {RichTextExtension} from '@lexical/rich-text'; +import * as Y from '@y/y'; +import { + $createParagraphNode, + $createRangeSelection, + $createTextNode, + $getRoot, + $isElementNode, + $isTextNode, + $setSelection, + configExtension, + defineExtension, + type LexicalEditor, + type LexicalNode, + type TextNode, + UNDO_COMMAND, +} from 'lexical'; +import {expect, test} from 'vitest'; + +import {createYDoc, YCollabExtension} from '../..'; +import {seedYState} from '../../headless'; + +const SEEDS = Number(process.env.US_SEEDS ?? 60); +const START = Number(process.env.US_START ?? 1); +const OPS = Number(process.env.US_OPS ?? 30); +const UNDO = Number(process.env.US_UNDO ?? 0.15); +const BLOCKS = 6; +const CONFIG = { + extensions: [RichTextExtension, MarkStateExtension], + schemaVersion: 1, +}; + +function rng(seed: number) { + let s = seed >>> 0 || 1; + return () => { + s ^= s << 13; + s ^= s >>> 17; + s ^= s << 5; + return (s >>> 0) / 4294967296; + }; +} + +type Op = {e: number} & ( + | {k: 'add'; t: number; a: number; b: number; id: string} + | {k: 'remove'; id: string} + | {k: 'format'; t: number; a: number; b: number; f: 'bold' | 'italic'} + | {k: 'type'; t: number; a: number; s: string} + | {k: 'del'; t: number; a: number; b: number} + | {k: 'undo'} +); + +function $texts(): TextNode[] { + const out: TextNode[] = []; + const walk = (n: LexicalNode) => { + if ($isTextNode(n) && n.getType() === 'text') { + out.push(n); + } + if ($isElementNode(n)) { + n.getChildren().forEach(walk); + } + }; + walk($getRoot()); + return out; +} + +function $select(t: TextNode, a: number, b: number) { + const s = $createRangeSelection(); + const n = t.getTextContentSize(); + s.anchor.set(t.getKey(), Math.min(a, n), 'text'); + s.focus.set(t.getKey(), Math.min(b, n), 'text'); + $setSelection(s); + return s; +} + +function $apply(op: Op) { + if (op.k === 'undo') { + return; + } + if (op.k === 'remove') { + $removeMarkID(op.id); + return; + } + const ts = $texts(); + const t = ts[op.t % Math.max(1, ts.length)]; + if (t === undefined) { + return; + } + const size = t.getTextContentSize(); + if (op.k === 'add') { + if (Math.min(op.b, size) > Math.min(op.a, size)) { + $addMarkID(op.id, $select(t, op.a, op.b)); + } + } else if (op.k === 'format') { + $select(t, op.a, op.b).formatText(op.f); + } else if (op.k === 'type') { + const s = $select(t, op.a, op.a); + s.format = 0; + s.insertText(op.s); + } else if (Math.min(op.b, size) > Math.min(op.a, size)) { + $select(t, op.a, op.b).removeText(); + } + $setSelection(null); +} + +/** The comment ids that are on more than one block. */ +function spread(editor: LexicalEditor): string[] { + return editor.read(() => { + const blocks = new Map>(); + for (const t of $texts()) { + const block = t.getParentOrThrow().getKey(); + for (const id of $getNodeMarkIDs(MarkStateExtension, t)) { + const set = blocks.get(id) ?? new Set(); + set.add(block); + blocks.set(id, set); + } + } + return [...blocks].filter(([, s]) => s.size > 1).map(([id]) => id); + }); +} + +function gen(seed: number): Op[] { + const r = rng(seed * 2654435761); + const ops: Op[] = []; + const live: string[] = []; + let n = 0; + const pos = () => Math.floor(r() * 30); + for (let i = 0; i < OPS; i++) { + const e = Math.floor(r() * 2); + const v = r() * (0.88 + UNDO); + const t = Math.floor(r() * BLOCKS * 3); + if (v < 0.25) { + const a = pos(); + const id = `c${++n}`; + live.push(id); + ops.push({a, b: a + 1 + Math.floor(r() * 12), e, id, k: 'add', t}); + } else if (v < 0.37 && live.length > 0) { + const [id] = live.splice(Math.floor(r() * live.length), 1); + ops.push({e, id, k: 'remove'}); + } else if (v < 0.5) { + const a = pos(); + const f = r() < 0.5 ? 'bold' : 'italic'; + ops.push({a, b: a + 1 + Math.floor(r() * 8), e, f, k: 'format', t}); + } else if (v < 0.68) { + const s = ['x', ' new ', 'yy'][Math.floor(r() * 3)]; + ops.push({a: pos(), e, k: 'type', s, t}); + } else if (v < 0.88) { + const a = pos(); + ops.push({a, b: a + 1 + Math.floor(r() * 6), e, k: 'del', t}); + } else { + ops.push({e, k: 'undo'}); + } + } + return ops; +} + +const WORDS = ['alpha', 'beta', 'gamma', 'delta', 'note', 'run']; + +// Seeds main fails (#9623), pinned as test.fails until the fix lands. +const FAILS_ON_MAIN = new Set([10, 12, 23, 26, 28, 35, 50, 51, 55]); + +for (let seed = START; seed < START + SEEDS; seed++) { + (FAILS_ON_MAIN.has(seed) ? test.fails : test)( + `an undo never spreads a comment onto another block (seed ${seed})`, + () => { + const seeded = seedYState(CONFIG, () => { + const root = $getRoot().clear(); + for (let i = 0; i < BLOCKS; i++) { + root.append( + $createParagraphNode().append( + $createTextNode(`${WORDS[i]} one two three four five ${i}`), + ), + ); + } + }); + const peers = ['A', 'B'].map(name => { + const doc = createYDoc(seeded); + const editor = buildEditorFromExtensions( + defineExtension({ + dependencies: [ + RichTextExtension, + MarkStateExtension, + configExtension(YCollabExtension, { + doc, + schemaVersion: 1, + synced: true, + }), + ], + name: `undo-spread-fuzz/${name}`, + onError: error => { + throw error; + }, + }), + ); + return {doc, editor}; + }); + const sync = () => { + const [a, b] = peers; + Y.applyUpdate( + b.doc, + Y.encodeStateAsUpdate(a.doc, Y.encodeStateVector(b.doc)), + ); + Y.applyUpdate( + a.doc, + Y.encodeStateAsUpdate(b.doc, Y.encodeStateVector(a.doc)), + ); + }; + const ops = gen(seed); + try { + ops.forEach((op, i) => { + const {editor} = peers[op.e]; + if (op.k === 'undo') { + editor.dispatchCommand(UNDO_COMMAND, undefined); + } else { + editor.update(() => $apply(op), {discrete: true}); + } + sync(); + for (const peer of peers) { + expect( + spread(peer.editor), + `after op ${i}: ${JSON.stringify(ops.slice(0, i + 1))}`, + ).toEqual([]); + } + }); + } finally { + for (const peer of peers) { + peer.editor.dispose(); + } + } + }, + ); +} diff --git a/packages/lexical-y/src/__tests__/unit/regressions/UndoCommentRemovalSpread.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/UndoCommentRemovalSpread.test.ts new file mode 100644 index 00000000000..d2f05bdc1b6 --- /dev/null +++ b/packages/lexical-y/src/__tests__/unit/regressions/UndoCommentRemovalSpread.test.ts @@ -0,0 +1,173 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + */ + +// @vitest-environment node + +// Two clients, synced after every step: A comments "two two " (c1) and +// maybe "two" (c2) and types at the start of the line, in one undo step; B +// removes c1; A undoes; B undoes its removal. c1 must come back on its own +// text only, never on the paragraphs after it (#9623). + +import {buildEditorFromExtensions} from '@lexical/extension'; +import { + $addMarkID, + $getNodeMarkIDs, + $removeMarkID, + MarkStateExtension, +} from '@lexical/mark'; +import {RichTextExtension} from '@lexical/rich-text'; +import * as Y from '@y/y'; +import { + $createParagraphNode, + $createRangeSelection, + $createTextNode, + $getRoot, + $isElementNode, + $isTextNode, + $setSelection, + configExtension, + defineExtension, + type LexicalEditor, + type RangeSelection, + UNDO_COMMAND, +} from 'lexical'; +import {describe, expect, it} from 'vitest'; + +import {createYDoc, YCollabExtension} from '../../..'; +import {readYState, seedYState} from '../../../headless'; + +const CONFIG = { + extensions: [RichTextExtension, MarkStateExtension], + schemaVersion: 1, +}; +const PARAS = [ + 'zero zero zero', + 'one one one', + 'two two two', + 'three three three', + 'four four four', +]; + +function $show(): string[] { + return $getRoot() + .getChildren() + .map(b => + $isElementNode(b) + ? b + .getChildren() + .map(c => { + const ids = $isTextNode(c) + ? $getNodeMarkIDs(MarkStateExtension, c) + : []; + return `${c.getTextContent()}${ids.length ? `[${ids.join(',')}]` : ''}`; + }) + .join('|') + : '?', + ); +} + +function clients() { + const seeded = seedYState(CONFIG, () => { + $getRoot() + .clear() + .append( + ...PARAS.map(t => $createParagraphNode().append($createTextNode(t))), + ); + }); + const [A, B] = ['A', 'B'].map(name => { + const doc = createYDoc(); + Y.applyUpdate(doc, seeded); + const editor = buildEditorFromExtensions( + defineExtension({ + dependencies: [ + RichTextExtension, + MarkStateExtension, + configExtension(YCollabExtension, { + doc, + schemaVersion: 1, + synced: true, + }), + ], + name: `repro/${name}`, + }), + ); + return {doc, editor}; + }) as unknown as [ + {doc: Y.Doc; editor: LexicalEditor}, + {doc: Y.Doc; editor: LexicalEditor}, + ]; + const sync = () => { + Y.applyUpdate( + B.doc, + Y.encodeStateAsUpdate(A.doc, Y.encodeStateVector(B.doc)), + ); + Y.applyUpdate( + A.doc, + Y.encodeStateAsUpdate(B.doc, Y.encodeStateVector(A.doc)), + ); + }; + return {A, B, sync}; +} + +function onRange( + editor: LexicalEditor, + a: number, + b: number, + f: (s: RangeSelection) => void, +) { + editor.update( + () => { + const t = ( + $getRoot().getChildAtIndex(2) as ReturnType + ).getFirstChild(); + if (!$isTextNode(t)) throw new Error('no text'); + const s = $createRangeSelection(); + s.anchor.set(t.getKey(), a, 'text'); + s.focus.set(t.getKey(), b, 'text'); + $setSelection(s); + f(s); + $setSelection(null); + }, + {discrete: true}, + ); +} + +describe('undoing the removal of a comment after its author undid adding it, with a second comment and typing', () => { + for (const typed of [true, false]) { + for (const second of [true, false]) { + // Fails on main (#9623): pinned until the fix lands. + it.fails( + `keeps the comment off the paragraphs after it (second comment: ${second}, typed at its start: ${typed})`, + () => { + const {A, B, sync} = clients(); + onRange(A.editor, 0, 8, s => $addMarkID('c1', s)); + if (second) onRange(A.editor, 0, 3, s => $addMarkID('c2', s)); + if (typed) + onRange(A.editor, 0, 0, s => { + s.format = 0; + s.insertText(' new '); + }); + sync(); + B.editor.update(() => $removeMarkID('c1'), {discrete: true}); + sync(); + A.editor.dispatchCommand(UNDO_COMMAND, undefined); + sync(); + B.editor.dispatchCommand(UNDO_COMMAND, undefined); + sync(); + const a = A.editor.getEditorState().read($show); + const b = B.editor.getEditorState().read($show); + const fresh = readYState(Y.encodeStateAsUpdate(A.doc), CONFIG, $show); + expect(b).toEqual(a); + expect(fresh).toEqual(a); + expect(a.slice(0, 2)).toEqual(PARAS.slice(0, 2)); + expect(a.slice(3)).toEqual(PARAS.slice(3)); + }, + ); + } + } +}); diff --git a/packages/lexical-y/src/__tests__/unit/regressions/UndoKeepsLaterFormat.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/UndoKeepsLaterFormat.test.ts new file mode 100644 index 00000000000..95ab3fa87cb --- /dev/null +++ b/packages/lexical-y/src/__tests__/unit/regressions/UndoKeepsLaterFormat.test.ts @@ -0,0 +1,94 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + */ +// @vitest-environment node + +// Three clients, synced after every step: one types, another deletes text +// next to it, the first makes a range italic, and the second undoes its +// delete. The undo brings back the deleted character and must keep the +// italic a collaborator gave the text around it afterwards (a fix that ends +// the runs an undo brings back where they ended at the delete would cut it). +import { + $createParagraphNode, + $createTextNode, + $getRoot, + type TextNode, +} from 'lexical'; +import {expect, test} from 'vitest'; + +import {$block, $selectIn, expectConverged, setup} from '../../utils/harness'; +import {prepareMarkState as prepare} from '../../utils/sizeOps'; + +const words = [ + ['w0abc', 'w1abc', 'w2ab', 'w3ab', 'w4ab'], + ['w5abc', 'w6ab', 'w7a', 'w8ab', 'w9abcd', 'w10abcd'], +]; +const preFmt = [ + [8, 2, 0, 1, 0], + [1, 0, 0, 0, 0, 0], +]; +function $plain(l: number, off: number, text: string) { + for (const node of $block([l]).getAllTextNodes() as TextNode[]) { + const size = node.getTextContentSize(); + if (off > 0 && off < size) { + const [, tail] = node.splitText(off); + tail.insertBefore($createTextNode(text)); + return; + } + if (off === size) { + node.insertAfter($createTextNode(text)); + return; + } + off -= size; + } +} + +test("an undo of a delete keeps a collaborator's later italic on live text", async () => { + const t = await setup( + () => { + words.forEach((ws, l) => { + const p = $createParagraphNode(); + ws.forEach((w, i) => + p.append( + $createTextNode(w + (i < ws.length - 1 ? ' ' : '')).setFormat( + preFmt[l][i], + ), + ), + ); + $getRoot().append(p); + }); + $getRoot().append( + $createParagraphNode().append($createTextNode('tail sentinel line')), + ); + }, + {ids: [3, 2, 1], prepare}, + ); + for (const p of t.peers) p.binding.undoManager?.clear(); + t.peers[0].update(() => $plain(0, 11, 'xq2')); + t.peers[1].update(() => void $selectIn([0], 6, 19).removeText()); + t.peers[1].update(() => void $selectIn([0], 6, 7).removeText()); + t.peers[0].update(() => void $selectIn([0], 0, 10).formatText('italic')); + t.peers[1].undo(); + await t.sync(); + await expectConverged(t.peers); + // Peer 0 made "w0abc " .. "3ab " italic (step 4) while it was live; peer 1's undo + // only brings back the one character it deleted at step 3. Main and a build with no + // collapse at all agree: the italic stays where peer 0 put it. + const runs = t.peers[0].read(() => + ($block([0]).getAllTextNodes() as TextNode[]).map(n => [ + n.getTextContent(), + n.getFormat(), + ]), + ); + expect(runs).toEqual([ + ['w0abc ', 10], + [' ', 2], + ['w3ab', 3], + [' ', 1], + ['w4ab', 0], + ]); +}); diff --git a/packages/lexical-y/src/__tests__/unit/regressions/UndoRestoredRunEndExtents.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/UndoRestoredRunEndExtents.test.ts new file mode 100644 index 00000000000..88caf164a36 --- /dev/null +++ b/packages/lexical-y/src/__tests__/unit/regressions/UndoRestoredRunEndExtents.test.ts @@ -0,0 +1,311 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + */ +// @vitest-environment node + +// The value an undo brings back covers exactly what it covered before: +// a collaborator removes a value, its author undoes it (and may write the +// same key later), a third client may delete text around the value's end, +// and the collaborator then undoes the removal. On main, with garbage +// collection on, the restored value runs on past its end (#9623), and with +// many comment ids or a concurrent undo of the end's change too (#9620): +// those cells are pinned as test.fails until the fix lands. +import {$toggleLink, LinkNode} from '@lexical/link'; +import {$addMarkID, $getNodeMarkIDs, $removeMarkID} from '@lexical/mark'; +import {$patchStyleText} from '@lexical/selection'; +import { + $createParagraphNode, + $createTextNode, + $getRoot, + $getState, + $isTextNode, + $setState, + type ElementNode, + type TextNode, +} from 'lexical'; +import {expect, test} from 'vitest'; + +import { + $block, + $selectIn, + freshReader, + type Peer, + type Setup, + setup, +} from '../../utils/harness'; +import {markState, prepareMarkState} from '../../utils/sizeOps'; + +/** The cells main fails (#9623, #9620), pinned until the fix lands. */ +function failsOnMain(name: string): boolean { + return name.includes('gc=true') || name.startsWith('concurrent'); +} +function testOnMain(name: string, fn: () => Promise) { + (failsOnMain(name) ? test.fails : test)(name, fn); +} + +const STATE = markState('review9633'); +const TEXT = 'abcd Kilo bb cc Delta end'; +type Kind = + | 'comment' + | 'bold' + | 'italic' + | 'underline' + | 'color' + | 'state' + | 'link'; +function $give(kind: Kind, start: number, end: number, on: boolean, count = 1) { + const s = $selectIn([0], start, end); + if (kind === 'comment') { + for (let i = 0; i < count; i++) { + if (on) { + $addMarkID(`c${i}`, s); + } else { + $removeMarkID(`c${i}`, s); + } + } + } else if (kind === 'color') $patchStyleText(s, {color: on ? 'red' : null}); + else if (kind === 'state') { + for (const n of s.extract()) if ($isTextNode(n)) $setState(n, STATE, on); + } else if (kind === 'link') $toggleLink(on ? 'https://a.test' : null); + else s.formatText(kind, on ? {bold: 1, italic: 2, underline: 8}[kind] : 0); +} +function chars(peer: Peer, kind: Kind): [string, boolean][][] { + return peer.read(() => + $getRoot() + .getChildren() + .map(block => + (block as ElementNode).getAllTextNodes().flatMap((n: TextNode) => { + const on = + kind === 'comment' + ? $getNodeMarkIDs(n).includes('c0') + : kind === 'color' + ? n.getStyle().includes('red') + : kind === 'state' + ? $getState(n, STATE) + : kind === 'link' + ? n.getParents().some(parent => parent instanceof LinkNode) + : n.hasFormat(kind); + return [...n.getTextContent()].map((c): [string, boolean] => [c, on]); + }), + ), + ); +} +async function check( + t: Setup, + kind: Kind, + expected: string[], + label: string, + expectedText = [TEXT, 'untouched tail'], +) { + const got = chars(t.a, kind); + for (const p of t.peers) { + expect(p.doc.store.pendingStructs, label + ' pending structs').toBe(null); + expect(p.doc.store.pendingDs, label + ' pending deletes').toBe(null); + expect(chars(p, kind), label).toEqual(got); + } + expect( + got.map(cs => cs.map(([c]) => c).join('')), + label + ' text', + ).toEqual(expectedText); + const fresh = await freshReader(t.a); + try { + expect(chars(fresh, kind), label + ' fresh').toEqual(got); + } finally { + fresh.stop(); + } + expect( + got.map(cs => cs.map(([c, on]) => (on ? '^' : c)).join('')), + label, + ).toEqual(expected); +} +function mask(text: string, start: number, end: number) { + return text.slice(0, start) + '^'.repeat(end - start) + text.slice(end); +} +async function make(gc: boolean, ids: number[]) { + const t = await setup( + () => + $getRoot().append( + $createParagraphNode().append($createTextNode(TEXT)), + $createParagraphNode().append($createTextNode('untouched tail')), + ), + {ids, prepare: prepareMarkState}, + ); + for (const p of t.peers) p.doc.gc = gc; + const sync = t.sync; + t.sync = async () => { + const before = t.peers.map(p => p.doc.store.getClock(p.doc.clientID)); + await sync(); + expect( + t.peers.map(p => p.doc.store.getClock(p.doc.clientID)), + 'no writes while applying remote updates', + ).toEqual(before); + }; + return t; +} + +for (const gc of [false, true]) + for (const ids of [ + [1, 2, 3], + [3, 2, 1], + ]) + for (const kind of [ + 'comment', + 'bold', + 'italic', + 'underline', + 'color', + 'state', + 'link', + ] as Kind[]) { + for (const later of ['none', 'before-undo', 'after-undo'] as const) { + testOnMain( + `same client later write ${later} ${kind} gc=${gc} ids=${ids}`, + async () => { + const t = await make(gc, ids); + try { + t.b.update(() => $give(kind, 5, 9, true)); + await t.sync(); + t.a.update(() => $give(kind, 5, 9, false)); + await t.sync(); + if (later === 'before-undo') { + t.b.update(() => $give(kind, 16, 21, true)); + await t.sync(); + t.b.undo(); + await t.sync(); + } + t.b.undo(); + await t.sync(); + if (later === 'after-undo') { + t.b.update(() => $give(kind, 16, 21, true)); + await t.sync(); + } + t.a.undo(); + await t.sync(); + const exp = + later === 'after-undo' + ? mask(mask(TEXT, 16, 21), 5, 9) + : mask(TEXT, 5, 9); + await check(t, kind, [exp, 'untouched tail'], 'restored extent'); + } finally { + for (const p of t.peers) p.stop(); + } + }, + ); + } + for (const del of [ + [8, 10], + [8, 14], + [5, 10], + [5, 14], + [3, 14], + ]) { + testOnMain( + `deleted noted end ${kind} gc=${gc} ids=${ids} del=${del}`, + async () => { + const t = await make(gc, ids); + try { + t.b.update(() => $give(kind, 5, 9, true)); + await t.sync(); + t.a.update(() => $give(kind, 5, 9, false)); + await t.sync(); + t.b.undo(); + await t.sync(); + t.c.update( + () => void $selectIn([0], del[0], del[1]).removeText(), + ); + await t.sync(); + t.a.undo(); + await t.sync(); + const remaining = TEXT.slice(0, del[0]) + TEXT.slice(del[1]); + const marked = [...TEXT] + .map((c, i) => (i >= 5 && i < 9 ? '^' : c)) + .join(''); + await check( + t, + kind, + [ + marked.slice(0, del[0]) + marked.slice(del[1]), + 'untouched tail', + ], + remaining, + [remaining, 'untouched tail'], + ); + } finally { + for (const p of t.peers) p.stop(); + } + }, + ); + } + } + +for (const count of [1, 16, 64, 128, 256]) + for (const gc of [false, true]) + testOnMain(`many comment keys count=${count} gc=${gc}`, async () => { + const t = await make(gc, [1, 2, 3]); + try { + t.b.update(() => $give('comment', 5, 9, true, count)); + await t.sync(); + t.a.update(() => $give('comment', 5, 9, false, count)); + await t.sync(); + t.b.undo(); + await t.sync(); + t.a.undo(); + await t.sync(); + await check( + t, + 'comment', + [mask(TEXT, 5, 9), 'untouched tail'], + 'cardinality', + ); + for (const p of t.peers) expect(p.doc.store.pendingStructs).toBe(null); + } finally { + for (const p of t.peers) p.stop(); + } + }); + +for (const count of [1, 16, 64, 128, 256]) + for (const ids of [ + [1, 2], + [2, 1], + ]) + testOnMain( + `concurrent empty restore count=${count} ids=${ids}`, + async () => { + const t = await make(false, ids); + try { + t.a.update(() => void $addMarkID('keep', $selectIn([0], 5, 9))); + t.b.update(() => { + for (let i = 0; i < count; i++) + $addMarkID(`c${i}`, $selectIn([0], 0, 3)); + }); + t.b.update(() => { + const [, tail] = ( + $block([0]).getAllTextNodes()[0] as TextNode + ).splitText(2); + tail.insertBefore($createTextNode('XY')); + }); + await t.sync(); + t.offline(); + t.a.update(() => void $selectIn([0], 2, 5).removeText()); + t.a.update(() => void $selectIn([0], 2, 7).removeText()); + t.a.undo(); + t.b.undo(); + t.b.undo(); + await t.sync(); + const remaining = 'abd Kilo bb cc Delta end'; + await check( + t, + 'comment', + [remaining, 'untouched tail'], + 'no undone comments', + [remaining, 'untouched tail'], + ); + } finally { + for (const p of t.peers) p.stop(); + } + }, + ); diff --git a/packages/lexical-y/src/__tests__/unit/regressions/UndoRestoredRunEnds.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/UndoRestoredRunEnds.test.ts new file mode 100644 index 00000000000..44564c04dae --- /dev/null +++ b/packages/lexical-y/src/__tests__/unit/regressions/UndoRestoredRunEnds.test.ts @@ -0,0 +1,440 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + */ + +// @vitest-environment node + +import {buildEditorFromExtensions} from '@lexical/extension'; +import { + $addMarkID, + $getNodeMarkIDs, + $removeMarkID, + MarkStateExtension, +} from '@lexical/mark'; +import {RichTextExtension} from '@lexical/rich-text'; +import {$patchStyleText} from '@lexical/selection'; +import * as Y from '@y/y'; +import { + $createParagraphNode, + $createRangeSelection, + $createTextNode, + $getRoot, + $getState, + $isElementNode, + $isTextNode, + $setSelection, + $setState, + configExtension, + defineExtension, + type ElementNode, + type LexicalEditor, + type LexicalNode, + REDO_COMMAND, + type TextNode, + UNDO_COMMAND, +} from 'lexical'; +import {describe, expect, test} from 'vitest'; + +import {createYDoc, YCollabExtension, type YDoc} from '../../..'; +import {readYState, seedYState} from '../../../headless'; +import {$block, $selectIn, expectConverged, setup} from '../../utils/harness'; +import {markState, prepareMarkState as prepare} from '../../utils/sizeOps'; + +// #9623, #9620: an undo that brings back an item opening a value (a comment, +// a format, a style, a NodeState value) whose run's end is gone, because +// another change's undo deleted it before (#9623) or at the same time +// (#9620), left the value open: it covered everything after it. The undo +// now ends the run it brings back where that run ended when the item was +// deleted, with a null of its own (see Binding#closeRestoredRuns and +// #recordRunEnds). +// +// Input space: the value is a comment id (MarkState), bold, a color style or +// a NodeState value; a collaborator removes it from the whole word, from part +// of it, or (comments) from the whole document; the value's author undoes +// its change before the removal is undone, or not; another client types +// plain text right after the word, or not; then a redo and another undo; +// garbage collection on and off; two client id orders. + +const PARAS = ['zero zero', 'one one', 'two two two', 'three three', 'four']; +const WORD: [block: number, start: number, end: number] = [2, 0, 5]; +const NODE_STATE = markState('n'); + +type Value = 'comment' | 'bold' | 'color' | 'NodeState'; +type Removal = 'the word' | 'part of the word' | 'the whole document'; + +function $wordSelection(start = WORD[1], end = WORD[2]) { + const block = $getRoot().getChildAtIndex(WORD[0]) as ElementNode; + let offset = 0; + const selection = $createRangeSelection(); + for (const node of block.getAllTextNodes()) { + const size = node.getTextContentSize(); + if (start >= offset && start <= offset + size) { + selection.anchor.set(node.getKey(), start - offset, 'text'); + break; + } + offset += size; + } + offset = 0; + for (const node of block.getAllTextNodes()) { + const size = node.getTextContentSize(); + if (end >= offset && end <= offset + size) { + selection.focus.set(node.getKey(), end - offset, 'text'); + break; + } + offset += size; + } + $setSelection(selection); + return selection; +} + +function $give(value: Value, on: boolean, start?: number, end?: number) { + const selection = $wordSelection(start, end); + if (value === 'comment') { + if (on) { + $addMarkID('c2', selection); + } else { + $removeMarkID('c2', selection); + } + } else if (value === 'bold') { + // 1 is bold's format flag: bold on, or off, whatever the text has. + selection.formatText('bold', on ? 1 : 0); + } else if (value === 'color') { + $patchStyleText(selection, {color: on ? 'red' : null}); + } else { + for (const node of selection.extract()) { + if ($isTextNode(node)) { + $setState(node, NODE_STATE, on); + } + } + } + $setSelection(null); +} + +function $has(value: Value, node: TextNode): boolean { + switch (value) { + case 'comment': + return $getNodeMarkIDs(MarkStateExtension, node).includes('c2'); + case 'bold': + return node.hasFormat('bold'); + case 'color': + return node.getStyle().includes('red'); + case 'NodeState': + return $getState(node, NODE_STATE); + } +} + +/** Each block's text with every character that has the value as `^`. */ +function $marked(value: Value): string[] { + return $getRoot() + .getChildren() + .map(block => + $isElementNode(block) + ? block + .getAllTextNodes() + .map(node => + $has(value, node) + ? '^'.repeat(node.getTextContentSize()) + : node.getTextContent(), + ) + .join('') + : '?', + ); +} + +const CONFIG = { + extensions: [RichTextExtension, MarkStateExtension], + schemaVersion: 1, +}; + +function clients(gc: boolean, ids: number[]) { + const seeded = seedYState(CONFIG, () => { + $getRoot() + .clear() + .append( + ...PARAS.map(text => + $createParagraphNode().append($createTextNode(text)), + ), + ); + }); + const peers = ids.map(id => { + const doc = createYDoc(seeded, {gc}); + doc.clientID = id; + const editor = buildEditorFromExtensions( + defineExtension({ + dependencies: [ + RichTextExtension, + MarkStateExtension, + configExtension(YCollabExtension, { + captureTimeout: 0, + doc, + schemaVersion: 1, + synced: true, + }), + ], + name: `undo-restored-run-ends/${id}`, + onError: error => { + throw error; + }, + }), + ); + return {doc, editor}; + }); + const sync = () => { + for (const from of peers) { + for (const to of peers) { + if (from !== to) { + Y.applyUpdate( + to.doc, + Y.encodeStateAsUpdate(from.doc, Y.encodeStateVector(to.doc)), + ); + } + } + } + }; + return {peers, sync}; +} + +function update(editor: LexicalEditor, fn: () => void) { + editor.update(fn, {discrete: true}); +} + +function liveFormatItems(doc: YDoc): number { + let n = 0; + const root = doc.get('lexical-y') as unknown as {_start: Y.Item | null}; + for (let it = root._start; it !== null; it = it.right) { + if (!it.deleted && it.content instanceof Y.ContentFormat) { + n++; + } + } + return n; +} + +/** + * Every client and a fresh read of the document agree, the other blocks have + * no value, and the word's block (with `^` where the value is) is + * `expected`. + */ +function check(value: Value, t: ReturnType, expected: string) { + const views = t.peers.map(p => p.editor.read(() => $marked(value))); + const fresh = readYState(Y.encodeStateAsUpdate(t.peers[0].doc), CONFIG, () => + $marked(value), + ); + for (const view of views) { + expect(view).toEqual(views[0]); + } + expect(fresh).toEqual(views[0]); + views[0].forEach((text, i) => { + if (i !== WORD[0]) { + expect(text).not.toContain('^'); + } + }); + expect(views[0][WORD[0]]).toBe(expected); +} + +/** + * The word's block with `^` exactly on `[a, b)` of it (nothing marked where + * `a === b`), and the text typed after the word, when there is some, unmarked. + */ +function exactly(a: number, b: number, typedAfter: boolean) { + const text = PARAS[WORD[0]]; + const marked = text.slice(0, a) + '^'.repeat(b - a) + text.slice(b); + const at = WORD[2]; + const expected = typedAfter + ? marked.slice(0, at) + 'ZZ' + marked.slice(at) + : marked; + return expected; +} + +const VALUES: Value[] = ['comment', 'bold', 'color', 'NodeState']; +const REMOVALS: Removal[] = [ + 'the word', + 'part of the word', + 'the whole document', +]; + +/** + * The cells main fails, pinned as test.fails until the fixes land: with + * garbage collection on, the restored value runs on past its end (#9623); + * where the removal took part of the word, the author's undo leaves the rest + * of the word with the value the removal restated after that part (D64, + * #9560), and the removal's undo gives the whole word back. + */ +function testOnMain(name: string, fn: () => void) { + const fails = + name.includes('its author undoes it first') && + (name.includes('gc on') || name.includes('part of the word')); + (fails ? test.fails : test)(name, fn); +} + +describe('an undo that brings back a value ends its run where it ended (#9623)', () => { + for (const gc of [false, true]) { + for (const ids of [ + [1, 2, 3], + [3, 2, 1], + ]) { + for (const value of VALUES) { + for (const removal of REMOVALS) { + if (removal === 'the whole document' && value !== 'comment') { + continue; + } + for (const authorUndoes of [true, false]) { + for (const typedAfter of [false, true]) { + testOnMain( + `${value}, removed from ${removal}, ${authorUndoes ? 'its author undoes it first' : 'kept by its author'}${typedAfter ? ', text typed after the word' : ''} (gc ${gc ? 'on' : 'off'}, ids ${ids})`, + () => { + const t = clients(gc, ids); + const [a, b, c] = t.peers; + update(b.editor, () => $give(value, true)); + t.sync(); + if (typedAfter) { + update(c.editor, () => { + const selection = $wordSelection(WORD[2], WORD[2]); + selection.format = 0; + selection.style = ''; + selection.insertNodes([$createTextNode('ZZ')]); + $setSelection(null); + }); + t.sync(); + } + update(a.editor, () => { + if (removal === 'the whole document') { + $removeMarkID('c2'); + } else if (removal === 'the word') { + $give(value, false); + } else { + $give(value, false, WORD[1], WORD[1] + 3); + } + }); + t.sync(); + if (authorUndoes) { + b.editor.dispatchCommand(UNDO_COMMAND, undefined); + t.sync(); + } + // What the removal took off the word, and what it left. + const [from, to] = + removal === 'part of the word' + ? [WORD[1], WORD[1] + 3] + : [WORD[1], WORD[2]]; + // The undo gives back what the removal took, and the author's + // undo already took the rest; a redo takes it again. + const undone = authorUndoes + ? exactly(from, to, typedAfter) + : exactly(WORD[1], WORD[2], typedAfter); + const redone = authorUndoes + ? exactly(0, 0, typedAfter) + : exactly(to, WORD[2], typedAfter); + a.editor.dispatchCommand(UNDO_COMMAND, undefined); + t.sync(); + check(value, t, undone); + a.editor.dispatchCommand(REDO_COMMAND, undefined); + t.sync(); + check(value, t, redone); + a.editor.dispatchCommand(UNDO_COMMAND, undefined); + t.sync(); + check(value, t, undone); + for (const p of t.peers) { + p.editor.dispose(); + } + }, + ); + } + } + } + } + } + } + + test('repeated undo and redo of the removal keep the items bounded', () => { + const t = clients(false, [1, 2, 3]); + const [a, b] = t.peers; + update(b.editor, () => $give('comment', true)); + t.sync(); + update(a.editor, () => $give('comment', false)); + t.sync(); + b.editor.dispatchCommand(UNDO_COMMAND, undefined); + t.sync(); + const counts: number[] = []; + for (let i = 1; i <= 40; i++) { + a.editor.dispatchCommand(UNDO_COMMAND, undefined); + t.sync(); + a.editor.dispatchCommand(REDO_COMMAND, undefined); + t.sync(); + if (i % 10 === 0) { + counts.push(liveFormatItems(a.doc)); + } + } + // The last step is a redo of the removal: no comment. + check('comment', t, exactly(0, 0, false)); + // As many live format items after 40 cycles as after 10. + expect(counts[3]).toBe(counts[0]); + for (const p of t.peers) { + p.editor.dispose(); + } + }); +}); + +// #9620: the end goes at the same time as the undo brings the value back. +describe('an undo that brings back a value while its end goes at the same time (#9620)', () => { + function show(peer: {read: (fn: () => T) => T}): string { + return peer.read(() => + $getRoot() + .getChildren() + .map((_, i) => + ($block([i]).getAllTextNodes() as TextNode[]) + .map(n => { + const ids = $getNodeMarkIDs(n as LexicalNode); + return ( + JSON.stringify(n.getTextContent()) + + (ids.length > 0 ? `[${ids.join(',')}]` : '') + ); + }) + .join(''), + ) + .join(' / '), + ); + } + for (const ids of [ + [1, 2], + [2, 1], + ]) { + // Fails on main (#9620): pinned until the fix lands. + test.fails( + `a comment its author undid does not cover the rest of the document (ids ${ids})`, + async () => { + const t = await setup( + () => + $getRoot().append( + $createParagraphNode().append($createTextNode('abcd Kilo bb cc')), + $createParagraphNode().append($createTextNode('tail line')), + ), + {ids, prepare}, + ); + t.a.update(() => void $addMarkID('c1', $selectIn([0], 5, 9))); + t.b.update(() => void $addMarkID('c2', $selectIn([0], 0, 3))); + t.b.update(() => { + const [, tail] = ( + $block([0]).getAllTextNodes() as TextNode[] + )[0].splitText(2); + tail.insertBefore($createTextNode('XY')); + }); + await t.sync(); + expect(show(t.a)).toBe( + '"ab"[c2]"XY""c"[c2]"d ""Kilo"[c1]" bb cc" / "tail line"', + ); + t.offline(); + t.a.update(() => void $selectIn([0], 2, 5).removeText()); + t.a.update(() => void $selectIn([0], 2, 7).removeText()); + t.a.undo(); + t.b.undo(); + t.b.undo(); + await t.sync(); + await expectConverged(t.peers); + expect(show(t.a)).toBe('"abd ""Kilo"[c1]" bb cc" / "tail line"'); + }, + ); + } +}); From d64ce422054c9ddc317888dea05725c7bfccc6c6 Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Fri, 30 Oct 2026 18:10:43 -0700 Subject: [PATCH 2/6] [lexical-y] Chore: Check every comment id in the many-id restored-extent cases ## Description The cases with 1 to 256 comment ids in one write checked the extent of c0 only, so a decoder that kept one id per character passed them. They now check that the restored word has every id. ## Test plan main (1853707e): UndoRestoredRunEndExtents 117 passed, 127 expected fail (244). --- .../unit/regressions/UndoRestoredRunEndExtents.test.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/packages/lexical-y/src/__tests__/unit/regressions/UndoRestoredRunEndExtents.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/UndoRestoredRunEndExtents.test.ts index 88caf164a36..79788c465a7 100644 --- a/packages/lexical-y/src/__tests__/unit/regressions/UndoRestoredRunEndExtents.test.ts +++ b/packages/lexical-y/src/__tests__/unit/regressions/UndoRestoredRunEndExtents.test.ts @@ -261,6 +261,16 @@ for (const count of [1, 16, 64, 128, 256]) [mask(TEXT, 5, 9), 'untouched tail'], 'cardinality', ); + // Every comment id comes back, not only c0. + const restored = t.a.read(() => + $block([0]) + .getAllTextNodes() + .filter(n => n.getTextContent().includes('Kilo')) + .map(n => [...$getNodeMarkIDs(n)].sort()), + ); + expect(restored, 'every comment id').toEqual([ + Array.from({length: count}, (_, i) => `c${i}`).sort(), + ]); for (const p of t.peers) expect(p.doc.store.pendingStructs).toBe(null); } finally { for (const p of t.peers) p.stop(); From d1df272b8cb932493e01b5d73e61481fe448f691 Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Fri, 30 Oct 2026 18:16:42 -0700 Subject: [PATCH 3/6] [lexical-y] Chore: Check the text, the restored comment and the spread itself in the undo restore cases ## Description - UndoRestoredRunEnds checks every block's text too, so a change that loses a character is caught (it checked only where the value is). - UndoCommentRemovalSpread checks that c1 is back on its words, so an undo that drops the value no longer passes it. - UndoSpreadFuzz pins the seeds that spread on main only with its default profile (US_OPS and US_UNDO unchanged): another profile makes other ops from the same seeds. ## Test plan main (1853707e), the five files: 262 passed, 194 expected fail. `US_OPS=29`: no seed pinned, the seeds that spread fail as tests. --- .../src/__tests__/unit/UndoSpreadFuzz.test.ts | 10 +++++++--- .../UndoCommentRemovalSpread.test.ts | 14 ++++++++++++++ .../regressions/UndoRestoredRunEnds.test.ts | 17 +++++++++++++++++ 3 files changed, 38 insertions(+), 3 deletions(-) diff --git a/packages/lexical-y/src/__tests__/unit/UndoSpreadFuzz.test.ts b/packages/lexical-y/src/__tests__/unit/UndoSpreadFuzz.test.ts index 510c58a7599..6eb9f3e0c09 100644 --- a/packages/lexical-y/src/__tests__/unit/UndoSpreadFuzz.test.ts +++ b/packages/lexical-y/src/__tests__/unit/UndoSpreadFuzz.test.ts @@ -180,11 +180,15 @@ function gen(seed: number): Op[] { const WORDS = ['alpha', 'beta', 'gamma', 'delta', 'note', 'run']; -// Seeds main fails (#9623), pinned as test.fails until the fix lands. -const FAILS_ON_MAIN = new Set([10, 12, 23, 26, 28, 35, 50, 51, 55]); +// Seeds that spread a comment on main (#9623) with the default profile +// (US_OPS and US_UNDO unchanged), pinned as test.fails until +// the fix lands. Another profile makes other ops from the same seed, so it +// pins nothing. +const DEFAULT_PROFILE = OPS === 30 && UNDO === 0.15; +const SPREADS_ON_MAIN = new Set([10, 12, 23, 26, 28, 35, 50, 51, 55]); for (let seed = START; seed < START + SEEDS; seed++) { - (FAILS_ON_MAIN.has(seed) ? test.fails : test)( + (DEFAULT_PROFILE && SPREADS_ON_MAIN.has(seed) ? test.fails : test)( `an undo never spreads a comment onto another block (seed ${seed})`, () => { const seeded = seedYState(CONFIG, () => { diff --git a/packages/lexical-y/src/__tests__/unit/regressions/UndoCommentRemovalSpread.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/UndoCommentRemovalSpread.test.ts index d2f05bdc1b6..8344207a33a 100644 --- a/packages/lexical-y/src/__tests__/unit/regressions/UndoCommentRemovalSpread.test.ts +++ b/packages/lexical-y/src/__tests__/unit/regressions/UndoCommentRemovalSpread.test.ts @@ -166,6 +166,20 @@ describe('undoing the removal of a comment after its author undid adding it, wit expect(fresh).toEqual(a); expect(a.slice(0, 2)).toEqual(PARAS.slice(0, 2)); expect(a.slice(3)).toEqual(PARAS.slice(3)); + // And c1 is back on its own text (an undo that dropped the value + // would keep the other paragraphs clean too). + const restored = A.editor.getEditorState().read(() => { + const block = $getRoot().getChildAtIndex(2); + return ( + $isElementNode(block) && + block + .getAllTextNodes() + .some(n => + $getNodeMarkIDs(MarkStateExtension, n).includes('c1'), + ) + ); + }); + expect(restored, 'c1 is back on its words').toBe(true); }, ); } diff --git a/packages/lexical-y/src/__tests__/unit/regressions/UndoRestoredRunEnds.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/UndoRestoredRunEnds.test.ts index 44564c04dae..e4427830abd 100644 --- a/packages/lexical-y/src/__tests__/unit/regressions/UndoRestoredRunEnds.test.ts +++ b/packages/lexical-y/src/__tests__/unit/regressions/UndoRestoredRunEnds.test.ts @@ -219,6 +219,23 @@ function liveFormatItems(doc: YDoc): number { * `expected`. */ function check(value: Value, t: ReturnType, expected: string) { + // Every block keeps its text (and the word's block the text typed after + // the word, where there is some), whatever the value. + const lines = PARAS.map((line, i) => + i === WORD[0] && expected.includes('ZZ') + ? line.slice(0, WORD[2]) + 'ZZ' + line.slice(WORD[2]) + : line, + ); + for (const p of t.peers) { + expect( + p.editor.read(() => + $getRoot() + .getChildren() + .map(block => block.getTextContent()), + ), + 'the text', + ).toEqual(lines); + } const views = t.peers.map(p => p.editor.read(() => $marked(value))); const fresh = readYState(Y.encodeStateAsUpdate(t.peers[0].doc), CONFIG, () => $marked(value), From 2baf3a18dbb521c5050211bca0bc3d4cd352588e Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Fri, 30 Oct 2026 18:36:19 -0700 Subject: [PATCH 4/6] [lexical-y] Chore: End UndoSpreadFuzz's undo steps where the script says, not by the runner's speed ## Description The fuzz grouped ops into undo steps by the UndoManager's 500 ms capture window, so a slow runner split them differently and a pinned seed could pass. Undo steps now end where a second sequence from the seed says (after an op with chance `US_STEP_END`, default 0.1), with the capture window out of reach, and the seeds that spread a comment on main with the default profile are pinned again: 10, 23, 26, 35 and 51. ## Test plan main (1853707e), twice under load: UndoSpreadFuzz 55 passed, 5 expected fail (60); the five files: all pass or fail as expected. --- .../src/__tests__/unit/UndoSpreadFuzz.test.ts | 31 +++++++++++++++---- 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/packages/lexical-y/src/__tests__/unit/UndoSpreadFuzz.test.ts b/packages/lexical-y/src/__tests__/unit/UndoSpreadFuzz.test.ts index 6eb9f3e0c09..c617737a4cf 100644 --- a/packages/lexical-y/src/__tests__/unit/UndoSpreadFuzz.test.ts +++ b/packages/lexical-y/src/__tests__/unit/UndoSpreadFuzz.test.ts @@ -14,9 +14,13 @@ // plain typing, deletes and undos. A comment id on two blocks is a spread // (#9623: "X adds, Y removes, X undoes, Y undoes" put it on everything // after it). Env: US_SEEDS (default 60), US_START, US_OPS (default 30), -// US_UNDO (default 0.15). +// US_UNDO (default 0.15), US_STEP_END (default 0.1: the chance an undo +// step ends after an op). -import {buildEditorFromExtensions} from '@lexical/extension'; +import { + buildEditorFromExtensions, + getExtensionDependencyFromEditor, +} from '@lexical/extension'; import { $addMarkID, $getNodeMarkIDs, @@ -49,6 +53,7 @@ const SEEDS = Number(process.env.US_SEEDS ?? 60); const START = Number(process.env.US_START ?? 1); const OPS = Number(process.env.US_OPS ?? 30); const UNDO = Number(process.env.US_UNDO ?? 0.15); +const STEP_END = Number(process.env.US_STEP_END ?? 0.1); const BLOCKS = 6; const CONFIG = { extensions: [RichTextExtension, MarkStateExtension], @@ -181,11 +186,11 @@ function gen(seed: number): Op[] { const WORDS = ['alpha', 'beta', 'gamma', 'delta', 'note', 'run']; // Seeds that spread a comment on main (#9623) with the default profile -// (US_OPS and US_UNDO unchanged), pinned as test.fails until +// (US_OPS, US_UNDO and US_STEP_END unchanged), pinned as test.fails until // the fix lands. Another profile makes other ops from the same seed, so it // pins nothing. -const DEFAULT_PROFILE = OPS === 30 && UNDO === 0.15; -const SPREADS_ON_MAIN = new Set([10, 12, 23, 26, 28, 35, 50, 51, 55]); +const DEFAULT_PROFILE = OPS === 30 && UNDO === 0.15 && STEP_END === 0.1; +const SPREADS_ON_MAIN = new Set([10, 23, 26, 35, 51]); for (let seed = START; seed < START + SEEDS; seed++) { (DEFAULT_PROFILE && SPREADS_ON_MAIN.has(seed) ? test.fails : test)( @@ -209,6 +214,9 @@ for (let seed = START; seed < START + SEEDS; seed++) { RichTextExtension, MarkStateExtension, configExtension(YCollabExtension, { + // Undo steps end where the script says, whatever the + // runner's speed (see below). + captureTimeout: 2 ** 30, doc, schemaVersion: 1, synced: true, @@ -220,7 +228,11 @@ for (let seed = START; seed < START + SEEDS; seed++) { }, }), ); - return {doc, editor}; + const {binding} = getExtensionDependencyFromEditor( + editor, + YCollabExtension, + ).output; + return {binding, doc, editor}; }); const sync = () => { const [a, b] = peers; @@ -234,6 +246,10 @@ for (let seed = START; seed < START + SEEDS; seed++) { ); }; const ops = gen(seed); + // Where an undo step ends: after an op with this chance (as edits a + // person makes in a burst share one step), from its own sequence so + // the ops stay those of the seed. + const steps = rng(seed * 40503 + 1); try { ops.forEach((op, i) => { const {editor} = peers[op.e]; @@ -242,6 +258,9 @@ for (let seed = START; seed < START + SEEDS; seed++) { } else { editor.update(() => $apply(op), {discrete: true}); } + if (steps() < STEP_END) { + peers[op.e].binding.undoManager?.stopCapturing(); + } sync(); for (const peer of peers) { expect( From 5578d7cb409d9643f1f1e84477afe7a2dfd696b3 Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Fri, 30 Oct 2026 19:17:12 -0700 Subject: [PATCH 5/6] [lexical-y] Chore: Accept the exact extent or nothing where the value's author undid it, and end UndoCommentRemovalSpread's undo steps explicitly ## Description - Whether a collaborator's undo of a removal brings back a value its author undid is undecided (#9623 accepts either). Where the author undid the value, the restored-extent cases now accept the exact original extent or nothing, and still fail on anything more; where the author keeps it, the value must come back exactly (a new set of many-id cases checks every id). - UndoCommentRemovalSpread ends its undo steps where the test says (a large capture window and stopCapturing after each client's step), not by the runner's speed, and accepts c1 on exactly its words or nowhere. ## Test plan main (1853707e), the five files: 276 passed, 190 expected fail. --- .../UndoCommentRemovalSpread.test.ts | 54 +++++--- .../UndoRestoredRunEndExtents.test.ts | 130 +++++++++++------- .../regressions/UndoRestoredRunEnds.test.ts | 21 ++- 3 files changed, 137 insertions(+), 68 deletions(-) diff --git a/packages/lexical-y/src/__tests__/unit/regressions/UndoCommentRemovalSpread.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/UndoCommentRemovalSpread.test.ts index 8344207a33a..8cd3cd31acd 100644 --- a/packages/lexical-y/src/__tests__/unit/regressions/UndoCommentRemovalSpread.test.ts +++ b/packages/lexical-y/src/__tests__/unit/regressions/UndoCommentRemovalSpread.test.ts @@ -13,7 +13,10 @@ // removes c1; A undoes; B undoes its removal. c1 must come back on its own // text only, never on the paragraphs after it (#9623). -import {buildEditorFromExtensions} from '@lexical/extension'; +import { + buildEditorFromExtensions, + getExtensionDependencyFromEditor, +} from '@lexical/extension'; import { $addMarkID, $getNodeMarkIDs, @@ -88,6 +91,9 @@ function clients() { RichTextExtension, MarkStateExtension, configExtension(YCollabExtension, { + // Undo steps end where the test says, whatever the runner's + // speed (see step). + captureTimeout: 2 ** 30, doc, schemaVersion: 1, synced: true, @@ -96,10 +102,19 @@ function clients() { name: `repro/${name}`, }), ); - return {doc, editor}; + const {binding} = getExtensionDependencyFromEditor( + editor, + YCollabExtension, + ).output; + return { + doc, + editor, + // Ends the client's current undo step. + step: () => binding.undoManager?.stopCapturing(), + }; }) as unknown as [ - {doc: Y.Doc; editor: LexicalEditor}, - {doc: Y.Doc; editor: LexicalEditor}, + {doc: Y.Doc; editor: LexicalEditor; step: () => void}, + {doc: Y.Doc; editor: LexicalEditor; step: () => void}, ]; const sync = () => { Y.applyUpdate( @@ -152,8 +167,11 @@ describe('undoing the removal of a comment after its author undid adding it, wit s.format = 0; s.insertText(' new '); }); + // A's comments and typing are one undo step. + A.step(); sync(); B.editor.update(() => $removeMarkID('c1'), {discrete: true}); + B.step(); sync(); A.editor.dispatchCommand(UNDO_COMMAND, undefined); sync(); @@ -166,20 +184,24 @@ describe('undoing the removal of a comment after its author undid adding it, wit expect(fresh).toEqual(a); expect(a.slice(0, 2)).toEqual(PARAS.slice(0, 2)); expect(a.slice(3)).toEqual(PARAS.slice(3)); - // And c1 is back on its own text (an undo that dropped the value - // would keep the other paragraphs clean too). - const restored = A.editor.getEditorState().read(() => { + // And c1 is on exactly its words or nowhere: the comment's author + // undid it, and whether B's undo of its removal brings it back is + // undecided (it must not cover less, nor more). + const withC1 = A.editor.getEditorState().read(() => { const block = $getRoot().getChildAtIndex(2); - return ( - $isElementNode(block) && - block - .getAllTextNodes() - .some(n => - $getNodeMarkIDs(MarkStateExtension, n).includes('c1'), - ) - ); + return $isElementNode(block) + ? block + .getAllTextNodes() + .filter(n => + $getNodeMarkIDs(MarkStateExtension, n).includes('c1'), + ) + .map(n => n.getTextContent()) + .join('') + : '?'; }); - expect(restored, 'c1 is back on its words').toBe(true); + expect(['', 'two two '], 'c1 on its words or nowhere').toContain( + withC1, + ); }, ); } diff --git a/packages/lexical-y/src/__tests__/unit/regressions/UndoRestoredRunEndExtents.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/UndoRestoredRunEndExtents.test.ts index 79788c465a7..4a40ba4d4a0 100644 --- a/packages/lexical-y/src/__tests__/unit/regressions/UndoRestoredRunEndExtents.test.ts +++ b/packages/lexical-y/src/__tests__/unit/regressions/UndoRestoredRunEndExtents.test.ts @@ -7,7 +7,9 @@ */ // @vitest-environment node -// The value an undo brings back covers exactly what it covered before: +// The value an undo brings back covers exactly what it covered before, or, +// where its author undid it, nothing (whether the collaborator's undo brings +// back a value its author undid is undecided, #9623); it never covers more: // a collaborator removes a value, its author undoes it (and may write the // same key later), a third client may delete text around the value's end, // and the collaborator then undoes the removal. On main, with garbage @@ -41,7 +43,10 @@ import {markState, prepareMarkState} from '../../utils/sizeOps'; /** The cells main fails (#9623, #9620), pinned until the fix lands. */ function failsOnMain(name: string): boolean { - return name.includes('gc=true') || name.startsWith('concurrent'); + return ( + (name.includes('gc=true') && !name.includes('author keeps')) || + name.startsWith('concurrent') + ); } function testOnMain(name: string, fn: () => Promise) { (failsOnMain(name) ? test.fails : test)(name, fn); @@ -97,7 +102,9 @@ function chars(peer: Peer, kind: Kind): [string, boolean][][] { async function check( t: Setup, kind: Kind, - expected: string[], + // The first line and the tail with `^` where the value is, or several + // such outcomes any of which is right. + expected: string[] | string[][], label: string, expectedText = [TEXT, 'untouched tail'], ) { @@ -117,10 +124,12 @@ async function check( } finally { fresh.stop(); } - expect( - got.map(cs => cs.map(([c, on]) => (on ? '^' : c)).join('')), - label, - ).toEqual(expected); + const shown = got.map(cs => cs.map(([c, on]) => (on ? '^' : c)).join('')); + if (Array.isArray(expected[0])) { + expect(expected as string[][], label).toContainEqual(shown); + } else { + expect(shown, label).toEqual(expected); + } } function mask(text: string, start: number, end: number) { return text.slice(0, start) + '^'.repeat(end - start) + text.slice(end); @@ -185,11 +194,17 @@ for (const gc of [false, true]) } t.a.undo(); await t.sync(); - const exp = - later === 'after-undo' - ? mask(mask(TEXT, 16, 21), 5, 9) - : mask(TEXT, 5, 9); - await check(t, kind, [exp, 'untouched tail'], 'restored extent'); + // The later write where it was written, and the word or not. + const later_ = later === 'after-undo' ? mask(TEXT, 16, 21) : TEXT; + await check( + t, + kind, + [ + [mask(later_, 5, 9), 'untouched tail'], + [later_, 'untouched tail'], + ], + 'restored extent', + ); } finally { for (const p of t.peers) p.stop(); } @@ -228,8 +243,11 @@ for (const gc of [false, true]) t, kind, [ - marked.slice(0, del[0]) + marked.slice(del[1]), - 'untouched tail', + [ + marked.slice(0, del[0]) + marked.slice(del[1]), + 'untouched tail', + ], + [remaining, 'untouched tail'], ], remaining, [remaining, 'untouched tail'], @@ -244,38 +262,58 @@ for (const gc of [false, true]) for (const count of [1, 16, 64, 128, 256]) for (const gc of [false, true]) - testOnMain(`many comment keys count=${count} gc=${gc}`, async () => { - const t = await make(gc, [1, 2, 3]); - try { - t.b.update(() => $give('comment', 5, 9, true, count)); - await t.sync(); - t.a.update(() => $give('comment', 5, 9, false, count)); - await t.sync(); - t.b.undo(); - await t.sync(); - t.a.undo(); - await t.sync(); - await check( - t, - 'comment', - [mask(TEXT, 5, 9), 'untouched tail'], - 'cardinality', - ); - // Every comment id comes back, not only c0. - const restored = t.a.read(() => - $block([0]) - .getAllTextNodes() - .filter(n => n.getTextContent().includes('Kilo')) - .map(n => [...$getNodeMarkIDs(n)].sort()), - ); - expect(restored, 'every comment id').toEqual([ - Array.from({length: count}, (_, i) => `c${i}`).sort(), - ]); - for (const p of t.peers) expect(p.doc.store.pendingStructs).toBe(null); - } finally { - for (const p of t.peers) p.stop(); - } - }); + for (const authorKeeps of [false, true]) + testOnMain( + `many comment keys count=${count} gc=${gc}${authorKeeps ? ' author keeps' : ''}`, + async () => { + const t = await make(gc, [1, 2, 3]); + try { + t.b.update(() => $give('comment', 5, 9, true, count)); + await t.sync(); + t.a.update(() => $give('comment', 5, 9, false, count)); + await t.sync(); + if (!authorKeeps) { + t.b.undo(); + await t.sync(); + } + t.a.undo(); + await t.sync(); + await check( + t, + 'comment', + authorKeeps + ? [mask(TEXT, 5, 9), 'untouched tail'] + : [ + [mask(TEXT, 5, 9), 'untouched tail'], + [TEXT, 'untouched tail'], + ], + 'cardinality', + ); + // Every comment id comes back (where the author undid them, every + // one or none), never only some. + const restored = t.a.read(() => + $block([0]) + .getAllTextNodes() + .filter(n => n.getTextContent().includes('Kilo')) + .map(n => [...$getNodeMarkIDs(n)].sort()), + ); + const every = [ + Array.from({length: count}, (_, i) => `c${i}`).sort(), + ]; + if (authorKeeps) { + expect(restored, 'every comment id').toEqual(every); + } else { + expect([every, [[]]], 'every comment id or none').toContainEqual( + restored, + ); + } + for (const p of t.peers) + expect(p.doc.store.pendingStructs).toBe(null); + } finally { + for (const p of t.peers) p.stop(); + } + }, + ); for (const count of [1, 16, 64, 128, 256]) for (const ids of [ diff --git a/packages/lexical-y/src/__tests__/unit/regressions/UndoRestoredRunEnds.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/UndoRestoredRunEnds.test.ts index e4427830abd..6d3d3c9bb48 100644 --- a/packages/lexical-y/src/__tests__/unit/regressions/UndoRestoredRunEnds.test.ts +++ b/packages/lexical-y/src/__tests__/unit/regressions/UndoRestoredRunEnds.test.ts @@ -218,11 +218,18 @@ function liveFormatItems(doc: YDoc): number { * no value, and the word's block (with `^` where the value is) is * `expected`. */ -function check(value: Value, t: ReturnType, expected: string) { +function check( + value: Value, + t: ReturnType, + // The word's block with `^` where the value is, or several such outcomes + // any of which is right. + expected: string | string[], +) { + const outcomes = typeof expected === 'string' ? [expected] : expected; // Every block keeps its text (and the word's block the text typed after // the word, where there is some), whatever the value. const lines = PARAS.map((line, i) => - i === WORD[0] && expected.includes('ZZ') + i === WORD[0] && outcomes[0].includes('ZZ') ? line.slice(0, WORD[2]) + 'ZZ' + line.slice(WORD[2]) : line, ); @@ -249,7 +256,7 @@ function check(value: Value, t: ReturnType, expected: string) { expect(text).not.toContain('^'); } }); - expect(views[0][WORD[0]]).toBe(expected); + expect(outcomes).toContain(views[0][WORD[0]]); } /** @@ -336,10 +343,12 @@ describe('an undo that brings back a value ends its run where it ended (#9623)', removal === 'part of the word' ? [WORD[1], WORD[1] + 3] : [WORD[1], WORD[2]]; - // The undo gives back what the removal took, and the author's - // undo already took the rest; a redo takes it again. + // The undo gives back what the removal took (the author's + // undo already took the rest), or, where the author undid + // the value, possibly nothing (undecided, #9623); never + // more. A redo takes it again. const undone = authorUndoes - ? exactly(from, to, typedAfter) + ? [exactly(from, to, typedAfter), exactly(0, 0, typedAfter)] : exactly(WORD[1], WORD[2], typedAfter); const redone = authorUndoes ? exactly(0, 0, typedAfter) From 2f38465ed7ae10a81a4c130cc61628e5b53329c5 Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Fri, 30 Oct 2026 20:04:30 -0700 Subject: [PATCH 6/6] [lexical-y] Chore: Check every comment id on every character in the many-id restored-extent cases ## Description The many-id cases checked the extent of c0 and the set of ids on the word only, so an id other than c0 spreading over the rest of the line or the next one passed. They now check every character of every line: the word has the restored set (every id, or, where the author undid them, every id or none), and every other character has none. ## Test plan main (1853707e): UndoRestoredRunEndExtents 127 passed, 127 expected fail (254), as before. --- .../UndoRestoredRunEndExtents.test.ts | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/packages/lexical-y/src/__tests__/unit/regressions/UndoRestoredRunEndExtents.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/UndoRestoredRunEndExtents.test.ts index 4a40ba4d4a0..8148401a6f8 100644 --- a/packages/lexical-y/src/__tests__/unit/regressions/UndoRestoredRunEndExtents.test.ts +++ b/packages/lexical-y/src/__tests__/unit/regressions/UndoRestoredRunEndExtents.test.ts @@ -307,6 +307,25 @@ for (const count of [1, 16, 64, 128, 256]) restored, ); } + // Every id on exactly the word's characters (or none), and on no + // other character of any line. + t.a.read(() => { + const onWord = restored[0]; + $getRoot() + .getChildren() + .forEach((block, line) => { + let offset = 0; + for (const n of (block as ElementNode).getAllTextNodes()) { + const ids = [...$getNodeMarkIDs(n)].sort(); + for (let k = 0; k < n.getTextContentSize(); k++) { + expect(ids, `ids on line ${line} at ${offset}`).toEqual( + line === 0 && offset >= 5 && offset < 9 ? onWord : [], + ); + offset++; + } + } + }); + }); for (const p of t.peers) expect(p.doc.store.pendingStructs).toBe(null); } finally {