[zxch3n]

Summary

Importing history from a client that shared a peer id (#1118) used to panic in RichtextState::apply_diff with the doc locks held. That poisoned the doc, and dropping it aborted the process. In other cases the import returned Ok and scrambled the text. Now the import returns Err before anything is mutated. The doc stays unchanged and usable, and dropping it is fine.

• Reused ids → LoroError::UsedOpID { id } (existing variant: "ID {id} has been used. You should use a new PeerID"), naming the first conflicting id.
• A change that skips counters of its peer (all deps present, but its own p@c-1 missing) → DecodeError. It used to trip the AppDag::update_version_on_new_change assertion under the op log lock and abort the same way.

Covers import (updates and snapshot into a non-empty doc), import_batch and import_json_updates. From JS, doc.import now throws instead of trapping with RuntimeError: unreachable.

Rationale

An import trims the part of each change the doc already has, by version vector, and applies the rest on top. With a reused peer id, the "known" part is different history. The new OpLog::check_and_trim_known_part_of_changes (crates/loro-internal/src/oplog/known_history.rs) runs right after decode in import_changes_and_apply_delta_to_state_if_needed, before the preflight. So an error takes the existing early-exit path, which only rolls back the arena (#1125/#1159 conventions). No rollback scope or lock-order change is needed.

• If the import brings anything new, every imported change that overlaps local history is compared with it over the overlap.
  • Deps are compared at every change boundary of either side.
  • Ops are cut at every op boundary of either side and compared by meaning: arena offsets are ignored, and so is the sign of a one-atom delete. Both sides may split or merge the same history differently, so this avoids false rejections.
  • JSON imports skip value payloads, because JSON text does not round-trip NaN or binary.
• The decoders no longer trim, since the check needs the known part. A snapshot with nothing new still returns no changes, without cloning them.
• The skipped-counter check replays the main pass of import_changes_to_oplog on counter ends only. Pending changes already wait for their own predecessor.

Details, what counts as equal, and what cannot be caught: context/import-peer-id-reuse.md.

One existing test changed: movable_list_invalid_ops::forged_after_map_change wrote its forged change as 2@0, while base_with_concurrent_edit() already has a local 2@0. That is a reused id, now reported before the movable-list check the test targets. The forged peer is now 4.

Validation

• New crates/loro/tests/import_reused_peer_id.rs (9 tests): the import with a reused peer id panics and poisons the doc; dropping it can abort #1118 repro through updates, snapshot, import_batch and JSON; a same-length conflict that used to scramble; a conflicting known change followed by a new one; skipped counters (including deps satisfied within the same import); drop inside catch_unwind; and re-imports that must still succeed (piecewise vs merged change stores, every op kind including NaN and one-atom reversed deletes, overlapping export windows, shallow docs).
  • On main (6e3e3c0): the repro panics, then panic in a destructor during cleanup aborts the test binary. The same-length case imports Ok with scrambled text. The skipped-counter case aborts the same way.
• New crates/loro-wasm/tests/import_reused_peer_id.test.ts. On main's WASM build: unreachable trap, and a silently scrambled import (2 of 3 tests fail). Passes here.
• cargo test --no-fail-fast -p loro -p loro-internal: 1337 passed, 0 failed (after rebasing on fix: reject imported Text ops past the end of the text #1169).
• pnpm check clean.
• pnpm -C crates/loro-wasm test (vitest 36 files / 446 tests, tsc, deno, bun) passes.

Perf

crates/examples/examples/import_known_history_perf.rs on a doc with 10k changes. Best of 7, three alternating main/fix rounds on 892ed30, loadavg ≈ 9–10:

scenario	main	this PR
2k small blobs imported one by one	14.3 ms	14.1–14.3 ms
only the new change	49.6–49.8	49.6–49.9
re-sent tail + new change	49.4–49.6	49.7–50.0
all updates, all known (no-op)	6.4–6.5	6.7
snapshot, all known (no-op)	6.6–6.7	6.5–6.6
all updates + 1 new change	56.0–56.4	64.2–64.5
snapshot + 1 new change	56.4–56.7	65.2–65.6

Common paths are unchanged within noise. The one real cost (about +15%) is re-importing the entire known history together with new changes, because that whole overlap is now compared. That is the shape where the bug lives.

Follow-ups (not fixed here)

• Delta sync cannot be checked. When the sender exports from the receiver's version vector, the conflicting prefix is not in the import, so there is nothing to compare. Since fix: reject imported Text ops past the end of the text #1169, an out-of-range tail is rejected by validate_diff, but a tail that lands on valid positions still scrambles. Detecting that would need content hashes.
• A conflicting duplicate with nothing new is a no-op and is not reported (kept cheap on purpose).
• Same-peer changes that are concurrent but contiguous (e.g. p@2 with deps [] on top of a local p@0..=1) are still accepted silently. They do not crash.
• A skipped-counter change parked as pending and later unlocked is handled by the pending path, which waits for p@c-1. The pre-check does not model pending changes that the same import unlocks out of lamport order.
• Drop still panics on a poisoned LoroMutex after any other panic inside the locks (import() panics when a remote update depends on ops folded into a shallow snapshot; still on main after #1064, and the panic poisons the doc mutex #1068). This PR removes this trigger, not that failure mode.

Fixes #1118

🤖 Generated with Claude Code

[zxch3n]

Superseded by #1171 (same commit, now part of a combined issue-sweep PR; GitHub closed this one when the branch was renamed).

[github-actions]

WASM Size Report

• Original size: 3285.18 KB
• Gzipped size: 1092.08 KB
• Brotli size: 765.12 KB