fix(undo): trim empty front stack rows (#983)

Author: ljho01

.changeset/fix-undo-trim-empty-front-rows.md

@@ -0,0 +1,5 @@
+---
+"loro-crdt": patch
+---
+
+Fix panic in `UndoManager` when `maxUndoSteps` trimming encounters an empty front stack row left by a prior undo with remote diffs.

crates/loro-internal/src/undo.rs

@@ -442,18 +442,40 @@ impl Stack {
         self.size
     }
 
+    fn discard_empty_front_rows(&mut self) {
+        // Undo pop can leave an empty front row that only carries remote diffs.
+        // There is no older stack item for that diff to transform during trimming.
+        while self
+            .stack
+            .front()
+            .is_some_and(|(items, _)| items.is_empty())
+        {
+            self.stack.pop_front();
+        }
+    }
+
+    fn ensure_trailing_empty_row(&mut self) {
+        if self.stack.is_empty() {
+            self.stack
+                .push_back((VecDeque::new(), Arc::new(Mutex::new(Default::default()))));
+        }
+    }
+
     fn pop_front(&mut self) {
         if self.is_empty() {
             return;
         }
 
+        self.discard_empty_front_rows();
         self.size -= 1;
         let first = self.stack.front_mut().unwrap();
         let f = first.0.pop_front();
         assert!(f.is_some());
         if first.0.is_empty() {
             self.stack.pop_front();
         }
+
+        self.ensure_trailing_empty_row();
     }
 
     fn set_top_meta(&mut self, meta: UndoItemMeta) {

crates/loro-wasm/tests/undo.test.ts

@@ -80,6 +80,35 @@ describe("undo", () => {
     expect(doc.getText("text").length).toBe(100);
   });
 
+  test("max undo steps after remote update and undo", () => {
+    const doc = new LoroDoc();
+    doc.setPeerId(1);
+    const text = doc.getText("text");
+    const undo = new UndoManager(doc, { maxUndoSteps: 3, mergeInterval: 0 });
+
+    text.insert(0, "A");
+    doc.commit();
+
+    const remote = new LoroDoc();
+    remote.setPeerId(2);
+    remote.import(doc.export({ mode: "snapshot" }));
+    remote.getText("text").insert(0, "R");
+    remote.commit();
+
+    doc.import(remote.export({ mode: "update" }));
+    expect(undo.undo()).toBeTruthy();
+    expect(text.toString()).toBe("R");
+
+    for (let i = 0; i < 4; i++) {
+      text.insert(text.length, i.toString());
+      doc.commit();
+    }
+
+    expect(doc.toJSON()).toStrictEqual({
+      text: "R0123",
+    });
+  });
+
   test("Skip chosen events", () => {
     const doc = new LoroDoc();
     const undo = new UndoManager(doc, {

crates/loro/tests/issue.rs

@@ -256,6 +256,37 @@ fn test_undo_counter_after_remote_update_issue_905() {
     assert_eq!(counter_a.get_value(), 2.0);
 }
 
+#[test]
+fn undo_max_steps_trim_after_remote_undo_should_not_panic() -> Result<(), LoroError> {
+    let doc_a = LoroDoc::new();
+    doc_a.set_peer_id(1)?;
+    let text_a = doc_a.get_text("text");
+    let mut undo_manager = UndoManager::new(&doc_a);
+    undo_manager.set_merge_interval(0);
+    undo_manager.set_max_undo_steps(3);
+
+    text_a.insert(0, "A")?;
+    doc_a.commit();
+
+    let doc_b = LoroDoc::from_snapshot(&doc_a.export(ExportMode::Snapshot)?)?;
+    doc_b.set_peer_id(2)?;
+    let text_b = doc_b.get_text("text");
+    text_b.insert(0, "R")?;
+    doc_b.commit();
+
+    doc_a.import(&doc_b.export(ExportMode::all_updates())?)?;
+    assert!(undo_manager.undo()?);
+    assert_eq!(text_a.to_string(), "R");
+
+    for i in 0..4 {
+        text_a.insert(text_a.len_unicode(), &i.to_string())?;
+        doc_a.commit();
+    }
+
+    assert_eq!(undo_manager.undo_count(), 3);
+    Ok(())
+}
+
 #[test]
 fn import_twice() {
     let doc = LoroDoc::new();