Collaboration: Y.Text delta conversion advances one index for two documents, dropping text on concurrent edits
Bug Description
In packages/collaboration/src/browser/collaboration-instance.ts, the Y.YTextEvent observer inside registerModelUpdate converts a Yjs delta into Monaco edit operations using a single index:
if (delta.retain !== undefined) {
index += delta.retain;
} else if (delta.insert !== undefined) {
const pos = model.textEditorModel.getPositionAt(index);
...
index += insert.length;
} else if (delta.delete !== undefined) {
const pos = model.textEditorModel.getPositionAt(index);
const endPos = model.textEditorModel.getPositionAt(index + delta.delete);
...
}The operations are collected and applied together through pushChangesToModel, so every getPositionAt call addresses the pre-edit document.
A Yjs delta walks two cursors. retain advances in both the old and new documents, insert advances only in the new one, and delete advances only in the old one. Since positions here are resolved against the old document, the index should advance on retain and on delete, and stand still on insert.
The current code does the opposite: it advances on insert and stands still on delete. Every operation after the first insert or delete in a multi-change delta is therefore displaced.
Impact
Two failure modes follow, and neither is visible to the user.
Displaced ranges can overlap, which pushEditOperations rejects. pushChangesToModel catches that and logs it, so the model is silently left unmodified.
Once the model is stale, the next local keystroke is converted using offsets from the stale model and written straight into the shared document, and the 200 ms resync then makes every peer agree on the damaged content. Text is lost with no conflict shown to anyone.
Single-change deltas are unaffected, which is why the problem appears only under concurrent editing of the same line.
Measurement
Against 1.73.1, over 400 seeded multi-change remote deltas: 255 left the receiving editor disagreeing with the shared document, and 31 produced overlapping ranges that Monaco refused.
Version
Reproduced on 1.73.1. The conversion is unchanged on master at the time of writing.
A correct implementation already exists in a current dependency
open-collaboration-yjs, which @theia/collaboration already depends on, ships YTextChangeTracker.applyDelta and gets this right. From 0.3.0:
if (typeof op.retain === 'number') {
index += op.retain;
} else if (typeof op.insert === 'string') {
changes.push({ start: index, end: index, text: op.insert });
} else if (typeof op.delete === 'number') {
changes.push({ start: index, end: index + op.delete, text: '' });
// Increase the index by the number of characters deleted
// In the client, every following operation will still operate on the "old code"
// So we need to adjust the index to reflect that
index += op.delete;
}It stands still on insert and advances on delete, which is what pre-edit addressing requires.
open-collaboration-monaco consumes that path rather than converting deltas itself. @theia/collaboration is the only one of the three that hand-rolls the conversion inline, and it is the one that has the bug.
So this may be fixable by using the tracker that is already a dependency rather than by patching the loop. It is exported from the package index and its YTextChange shape maps directly onto the ranges built here.
Source: eclipse-theia/theia