Skip to content

🐛 Transform every pending op by each fixup op - #718

Merged
alecgibson merged 1 commit into
masterfrom
fix-fixup-transforms-wrong-pending-op
Aug 24, 2026
Merged

🐛 Transform every pending op by each fixup op#718
alecgibson merged 1 commit into
masterfrom
fix-fixup-transforms-wrong-pending-op

Conversation

@alecgibson

Copy link
Copy Markdown
Collaborator

Fixes #717

At the moment, when the server acknowledges an op carrying fixup ops, Doc._opAcknowledged() loops over pendingOps with an inner index j, but indexes the array with i — the outer loop variable, which enumerates the fixup ops instead.

Since transformX() mutates both of its arguments in place, this isn't a harmless redundant pass. We transform pendingOps[0] once per pending op rather than once, and never transform pendingOps[1] onwards at all, so the client and the server end up disagreeing about the order of the remaining pending ops. Worse, if there are more fixup ops than pending ops, we hand undefined to transformX(), which throws a TypeError that escapes all the way out of Connection.handleMessage().

The existing tests all miss this, since they have at most one fixup op and one pending op, where i and j are both always 0.

This change indexes pendingOps with the inner loop variable, and renames both variables to fixupIndex and pendingIndex so the two can't be confused again. The equivalent loop in Doc._handleOp() was already correct.

🤖 Generated with Claude Code

Co-Authored-By: Claude noreply@anthropic.com

Fixes #717

At the moment, when the server acknowledges an op carrying fixup ops,
`Doc._opAcknowledged()` loops over `pendingOps` with an inner index `j`,
but indexes the array with `i` — the outer loop variable, which
enumerates the fixup ops instead.

Since `transformX()` mutates both of its arguments in place, this isn't
a harmless redundant pass. We transform `pendingOps[0]` once per pending
op rather than once, and never transform `pendingOps[1]` onwards at all,
so the client and the server end up disagreeing about the order of the
remaining pending ops. Worse, if there are more fixup ops than pending
ops, we hand `undefined` to `transformX()`, which throws a `TypeError`
that escapes all the way out of `Connection.handleMessage()`.

The existing tests all miss this, since they have at most one fixup op
and one pending op, where `i` and `j` are both always `0`.

This change indexes `pendingOps` with the inner loop variable, and
renames both variables to `fixupIndex` and `pendingIndex` so the two
can't be confused again. The equivalent loop in `Doc._handleOp()` was
already correct.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@coveralls

Copy link
Copy Markdown

Coverage Status

coverage: 97.467%. remained the same — fix-fixup-transforms-wrong-pending-op into master

@alecgibson
alecgibson marked this pull request as ready for review July 31, 2026 13:26
@alecgibson
alecgibson merged commit 0bfe7d5 into master Aug 24, 2026
8 checks passed
@alecgibson
alecgibson deleted the fix-fixup-transforms-wrong-pending-op branch August 24, 2026 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Doc._opAcknowledged transforms the wrong pendingOps entry when handling fixup ops

3 participants