From 40270661ada7dfb55c30ae74074eedbbee13c656 Mon Sep 17 00:00:00 2001
From: Alec Gibson <12036746+alecgibson@users.noreply.github.com>
Date: Fri, 31 Jul 2026 11:42:50 +0000
Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Transform=20every=20pending=20op?=
 =?UTF-8?q?=20by=20each=20fixup=20op?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Fixes https://github.com/share/sharedb/issues/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>
---
 lib/client/doc.js  |  8 +++---
 test/middleware.js | 62 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+), 4 deletions(-)

diff --git a/lib/client/doc.js b/lib/client/doc.js
index dcd48ec9c..d3713492d 100644
--- a/lib/client/doc.js
+++ b/lib/client/doc.js
@@ -968,11 +968,11 @@ Doc.prototype._opAcknowledged = function(message) {
   }
 
   if (message[ACTIONS.fixup]) {
-    for (var i = 0; i < message[ACTIONS.fixup].length; i++) {
-      var fixupOp = message[ACTIONS.fixup][i];
+    for (var fixupIndex = 0; fixupIndex < message[ACTIONS.fixup].length; fixupIndex++) {
+      var fixupOp = message[ACTIONS.fixup][fixupIndex];
 
-      for (var j = 0; j < this.pendingOps.length; j++) {
-        var transformErr = transformX(this.pendingOps[i], fixupOp);
+      for (var pendingIndex = 0; pendingIndex < this.pendingOps.length; pendingIndex++) {
+        var transformErr = transformX(this.pendingOps[pendingIndex], fixupOp);
         if (transformErr) return this._hardRollback(transformErr);
       }
 
diff --git a/test/middleware.js b/test/middleware.js
index 66e50b2fa..6f87c2f87 100644
--- a/test/middleware.js
+++ b/test/middleware.js
@@ -565,6 +565,68 @@ describe('middleware', function() {
       });
     });
 
+    it('transforms a pending op by multiple fixups', function(done) {
+      var applied = false;
+      backend.use('apply', function(request, next) {
+        if (applied) return next();
+        applied = true;
+        request.$fixup([{p: ['tricks', 0], li: 'sit'}]);
+        request.$fixup([{p: ['tricks', 0], li: 'stay'}]);
+        next();
+      });
+
+      var remoteConnection = backend.connect();
+      var remoteDoc = remoteConnection.get('dogs', 'fido');
+
+      remoteDoc.subscribe(function(error) {
+        if (error) return done(error);
+
+        expect(remoteDoc.data).to.eql({name: 'fido'});
+
+        remoteDoc.on('op batch', function() {
+          if (remoteDoc.version !== 3) return;
+          expect(remoteDoc.data.tricks).to.eql(['stay', 'sit', 'fetch', 'lie down']);
+          expect(remoteDoc.data).to.eql(doc.data);
+          done();
+        });
+
+        doc.preventCompose = true;
+        doc.submitOp([{p: ['tricks'], oi: ['fetch']}], errorHandler(done));
+        doc.submitOp([{p: ['tricks', 1], li: 'lie down'}], errorHandler(done));
+      });
+    });
+
+    it('transforms multiple pending ops by a fixup', function(done) {
+      var applied = false;
+      backend.use('apply', function(request, next) {
+        if (applied) return next();
+        applied = true;
+        request.$fixup([{p: ['tricks', 0], li: 'stay'}]);
+        next();
+      });
+
+      var remoteConnection = backend.connect();
+      var remoteDoc = remoteConnection.get('dogs', 'fido');
+
+      remoteDoc.subscribe(function(error) {
+        if (error) return done(error);
+
+        expect(remoteDoc.data).to.eql({name: 'fido'});
+
+        remoteDoc.on('op batch', function() {
+          if (remoteDoc.version !== 4) return;
+          expect(remoteDoc.data.tricks).to.eql(['stay', 'fetch', 'sit', 'roll over']);
+          expect(remoteDoc.data).to.eql(doc.data);
+          done();
+        });
+
+        doc.preventCompose = true;
+        doc.submitOp([{p: ['tricks'], oi: ['fetch']}], errorHandler(done));
+        doc.submitOp([{p: ['tricks', 1], li: 'sit'}], errorHandler(done));
+        doc.submitOp([{p: ['tricks', 2], li: 'roll over'}], errorHandler(done));
+      });
+    });
+
     it('applies a fixup to a creation op', function(done) {
       backend.use('apply', function(request, next) {
         request.$fixup([{p: ['goodBoy'], oi: true}]);