Skip to content

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

Open
alecgibson wants to merge 1 commit into
masterfrom
fix-fixup-transforms-wrong-pending-op
Open

🐛 Transform every pending op by each fixup op#718
alecgibson wants to merge 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
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

2 participants