diff --git a/.changeset/mergeable-pending-import.md b/.changeset/mergeable-pending-import.md new file mode 100644 index 000000000..14369ce6e --- /dev/null +++ b/.changeset/mergeable-pending-import.md @@ -0,0 +1,5 @@ +--- +"loro-crdt": patch +--- + +Fix `unreachable` panic when importing an out-of-order update whose op targets a mergeable child container before its creation (or its parent map) has arrived. Such ops are now buffered as pending and applied once the creating change is imported. diff --git a/Cargo.lock b/Cargo.lock index c55e16bae..9c5390a31 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1873,7 +1873,7 @@ checksum = "3f3d053a135388e6b1df14e8af1212af5064746e9b87a06a345a7a779ee9695a" [[package]] name = "loro-wasm" -version = "1.13.1" +version = "1.13.2" dependencies = [ "console_error_panic_hook", "js-sys", diff --git a/crates/loro-internal/src/loro.rs b/crates/loro-internal/src/loro.rs index 5b476e17a..10d764466 100644 --- a/crates/loro-internal/src/loro.rs +++ b/crates/loro-internal/src/loro.rs @@ -2160,7 +2160,15 @@ fn pending_root_containers_to_materialize(oplog: &OpLog, changes: &[Change]) -> .arena .get_container_id(op.container) .expect("decoded op container should be registered"); - if id.is_root() { + // Mergeable containers share the `ContainerID::Root` namespace but are logical + // *children*: their existence is governed by their parent map's marker, and their + // parent can be any (possibly not-yet-imported) map. Eagerly materializing one + // while its causal dependencies are still pending has no valid parent edge to + // resolve its depth against, which used to panic in `ContainerWrapper::new`. + // They get materialized correctly through the normal diff path once the creating + // change applies, so skip them here (mirrors the `!is_mergeable()` guard in + // `ensure_root_container`). See the `mergeable_container::pending` regression test. + if id.is_root() && !id.is_mergeable() { roots.insert(id); } } diff --git a/crates/loro-internal/tests/mergeable_container.rs b/crates/loro-internal/tests/mergeable_container.rs index bbb6c4cc3..5ed9541f9 100644 --- a/crates/loro-internal/tests/mergeable_container.rs +++ b/crates/loro-internal/tests/mergeable_container.rs @@ -9,6 +9,8 @@ mod delete; mod discriminator; #[path = "mergeable_container/events_and_paths.rs"] mod events_and_paths; +#[path = "mergeable_container/pending.rs"] +mod pending; #[path = "mergeable_container/snapshot.rs"] mod snapshot; #[path = "mergeable_container/type_conflict.rs"] diff --git a/crates/loro-internal/tests/mergeable_container/pending.rs b/crates/loro-internal/tests/mergeable_container/pending.rs new file mode 100644 index 000000000..260f2ed5d --- /dev/null +++ b/crates/loro-internal/tests/mergeable_container/pending.rs @@ -0,0 +1,53 @@ +//! Importing ops on mergeable children whose causal dependencies (the child's creation +//! and/or its parent map) have not arrived yet must be buffered as pending, not panic. +//! +//! Regression for the `unreachable`/`Option::unwrap()` trap in `ContainerWrapper::new` when a +//! pending change referenced a mergeable container under a not-yet-imported parent map. Mergeable +//! cids live in the `ContainerID::Root` namespace, so the pending-root materialization path used +//! to treat them as ordinary roots and eagerly build their state — but a mergeable container is a +//! logical *child* whose depth cannot be resolved before its parent exists. + +#[path = "common.rs"] +mod common; +use common::doc; + +use loro_internal::{cursor::PosType, handler::MapHandler, loro::ExportMode}; + +#[test] +fn import_edit_on_mergeable_child_under_normal_map_before_creation() { + // root map -> Normal child map `m` -> mergeable text `body` under `m`. + // The mergeable child's parent is a Normal (non-root) container, so delivering the edit + // without the creation exercises the unresolved-depth path that used to panic. + let a = doc(1); + let root = a.get_map("root"); + let m = root + .insert_container("m", MapHandler::new_detached()) + .unwrap(); + a.commit_then_renew(); + let after_create = a.oplog_vv(); + + // Capture the creation-only payload *now*, before the edit exists, so the second import + // delivers strictly the missing dependency. If we exported it after the edit, it would also + // re-include the edit and the convergence assert could pass even if the pending edit was lost. + let creation_only = a.export(ExportMode::updates(&Default::default())).unwrap(); + + let body = m.ensure_mergeable_text("body").unwrap(); + body.insert(0, "hi", PosType::Unicode).unwrap(); + a.commit_then_renew(); + + // Only the edit commit, without the creation it causally depends on. + let edit_only = a.export(ExportMode::updates(&after_create)).unwrap(); + + let b = doc(2); + // Must buffer as pending instead of panicking, and actually retain the edit as pending. + let status = b.import(&edit_only).unwrap(); + assert!( + status.pending.is_some(), + "edit on a mergeable child whose creation is missing must be buffered as pending" + ); + + // Delivering the missing creation (which does NOT contain the edit) replays the pending + // edit and converges. + b.import(&creation_only).unwrap(); + assert_eq!(a.get_deep_value(), b.get_deep_value()); +}