fix(editor): tag inserts with clientVars author until userAuthor propagates (Firefox authorship flake)

src/static/js/collab_client.ts

@@ -26,6 +26,7 @@
 const chat = require('./chat').chat;
 const hooks = require('./pluginfw/hooks');
 const browser = require('./vendors/browser');
+import {stampAuthorOnInserts} from './stampAuthorOnInserts';
 
 // Dependency fill on init. This exists for `pad.socket` only.
 // TODO: bind directly to the socket.
@@ -48,6 +49,23 @@ const getCollabClient = (ace2editor, serverVars, initialUserInfo, options, _pad)
   let commitDelay = 500;
 
   const userId = initialUserInfo.userId;
+
+  // Build the outgoing changeset and guarantee every insert carries an author.
+  // collab_client only exists after CLIENT_VARS has arrived, so `userId` is always
+  // populated here — whereas the editor can build a changeset with an empty author
+  // if the user typed before CLIENT_VARS landed (the early-typing race that the
+  // server's pad-corruption guard rejects). Stamping here is the last line of
+  // defense before the changeset goes on the wire. See stampAuthorOnInserts.
+  const prepareUserChangeset = () => {
+    const data = editor.prepareUserChangeset();
+    if (data.changeset) {
+      const stamped = stampAuthorOnInserts(data.changeset, data.apool, userId);
+      data.changeset = stamped.changeset;
+      data.apool = stamped.apool;
+    }
+    return data;
+  };
+
   // var socket;
   const userSet = {}; // userId -> userInfo
   userSet[userId] = initialUserInfo;
@@ -114,7 +132,7 @@ const getCollabClient = (ace2editor, serverVars, initialUserInfo, options, _pad)
     // Check if there are any pending revisions to be received from server.
     // Allow only if there are no pending revisions to be received from server
     if (!isPendingRevision) {
-      const userChangesData = editor.prepareUserChangeset();
+      const userChangesData = prepareUserChangeset();
       if (userChangesData.changeset) {
         lastCommitTime = now;
         committing = true;
@@ -416,7 +434,7 @@ const getCollabClient = (ace2editor, serverVars, initialUserInfo, options, _pad)
       obj.committedChangesetAPool = stateMessage.apool;
       editor.applyPreparedChangesetToBase();
     }
-    const userChangesData = editor.prepareUserChangeset();
+    const userChangesData = prepareUserChangeset();
     if (userChangesData.changeset) {
       obj.furtherChangeset = userChangesData.changeset;
       obj.furtherChangesetAPool = userChangesData.apool;

src/static/js/stampAuthorOnInserts.ts

@@ -0,0 +1,58 @@
+'use strict';
+
+import {deserializeOps, pack, unpack} from './Changeset';
+import AttributeMap from './AttributeMap';
+import AttributePool from './AttributePool';
+import {SmartOpAssembler} from './SmartOpAssembler';
+
+/**
+ * Ensure every insert (`+`) op in a wire changeset carries an `author` attribute.
+ *
+ * Why this exists: the local author id (`clientVars.userId`) only becomes available
+ * once the CLIENT_VARS socket message arrives. Under load (notably Firefox with
+ * plugins) the editor can become editable and the user can type *before* that
+ * message lands, so the editor tags the insert with an empty author. An empty
+ * author canonicalizes to "no author", producing an unattributed insert that the
+ * server's pad-corruption guard rejects — dropping the whole USER_CHANGES and
+ * silently losing the typed text's authorship (the clear_authorship_color flake).
+ *
+ * This runs in collab_client, which only exists after CLIENT_VARS has arrived, so
+ * the author id passed here is always populated. Stamping any author-less insert
+ * just before the changeset is sent guarantees the server never sees an
+ * unattributed insert, independent of editor-init timing.
+ *
+ * @param changeset - The wire changeset string from prepareUserChangeset().
+ * @param apoolJsonable - The jsonable wire attribute pool that accompanies it.
+ * @param authorId - The local author id (collab_client's userId). If falsy, the
+ *     inputs are returned unchanged (nothing better to stamp with).
+ * @returns The (possibly) rewritten changeset + jsonable pool. Returns the inputs
+ *     unchanged when no insert needed an author, so the common path is a no-op.
+ */
+export const stampAuthorOnInserts = (
+  changeset: string,
+  apoolJsonable: any,
+  authorId: string,
+): {changeset: string, apool: any} => {
+  if (!authorId) return {changeset, apool: apoolJsonable};
+  const pool = (new AttributePool()).fromJsonable(apoolJsonable);
+  const unpacked = unpack(changeset);
+  const assem = new SmartOpAssembler();
+  let modified = false;
+  for (const op of deserializeOps(unpacked.ops)) {
+    if (op.opcode === '+') {
+      const attribs = AttributeMap.fromString(op.attribs, pool);
+      if (!attribs.get('author')) {
+        attribs.set('author', authorId);
+        op.attribs = attribs.toString();
+        modified = true;
+      }
+    }
+    assem.append(op);
+  }
+  if (!modified) return {changeset, apool: apoolJsonable};
+  assem.endDocument();
+  return {
+    changeset: pack(unpacked.oldLen, unpacked.newLen, assem.toString(), unpacked.charBank),
+    apool: pool.toJsonable(),
+  };
+};

src/tests/backend-new/specs/stampAuthorOnInserts.test.ts

@@ -0,0 +1,76 @@
+'use strict';
+
+import {describe, it, expect} from 'vitest';
+import {stampAuthorOnInserts} from '../../../static/js/stampAuthorOnInserts';
+import {checkRep, deserializeOps, unpack} from '../../../static/js/Changeset';
+import AttributeMap from '../../../static/js/AttributeMap';
+import AttributePool from '../../../static/js/AttributePool';
+
+const AUTHOR = 'a.test1234567890';
+const EMPTY_POOL = () => (new AttributePool()).toJsonable();
+
+// Read the author attribute off the first '+' op of a (changeset, jsonable pool).
+const firstInsertAuthor = (changeset: string, apoolJsonable: any): string | undefined => {
+  const pool = (new AttributePool()).fromJsonable(apoolJsonable);
+  for (const op of deserializeOps(unpack(changeset).ops)) {
+    if (op.opcode === '+') return AttributeMap.fromString(op.attribs, pool).get('author');
+  }
+  return undefined;
+};
+
+describe('stampAuthorOnInserts', () => {
+  it('stamps the author onto an unattributed insert (the flake changeset)', () => {
+    // `Z:1>5+5$Hello` — insert "Hello" with NO author, exactly what the editor
+    // emits during the early-typing race and what the server rejects.
+    const input = 'Z:1>5+5$Hello';
+    const {changeset, apool} = stampAuthorOnInserts(input, EMPTY_POOL(), AUTHOR);
+    // The insert now carries the author...
+    expect(firstInsertAuthor(changeset, apool)).toBe(AUTHOR);
+    // ...the result is a valid canonical changeset...
+    expect(() => checkRep(changeset)).not.toThrow();
+    // ...and the text is preserved.
+    expect(unpack(changeset).charBank).toBe('Hello');
+    // It actually changed (was unattributed before).
+    expect(changeset).not.toBe(input);
+  });
+
+  it('leaves an already-attributed insert unchanged', () => {
+    // Build `Z:1>5*0+5$Hello` with author already in the pool.
+    const pool = new AttributePool();
+    const n = pool.putAttrib(['author', AUTHOR]); // index 0
+    const attributed = `Z:1>5*${n}+5$Hello`;
+    const jsonable = pool.toJsonable();
+    const {changeset, apool} = stampAuthorOnInserts(attributed, jsonable, AUTHOR);
+    expect(changeset).toBe(attributed); // unchanged (no-op path)
+    expect(firstInsertAuthor(changeset, apool)).toBe(AUTHOR);
+  });
+
+  it('does not invent an author when authorId is empty', () => {
+    const input = 'Z:1>5+5$Hello';
+    const {changeset} = stampAuthorOnInserts(input, EMPTY_POOL(), '');
+    expect(changeset).toBe(input); // nothing to stamp with → unchanged
+  });
+
+  it('does not touch keep/remove-only changesets', () => {
+    // `Z:6<1=5-1$` — keep 5, remove 1; no insert ops.
+    const input = 'Z:6<1=5-1$x';
+    const {changeset} = stampAuthorOnInserts(input, EMPTY_POOL(), AUTHOR);
+    expect(changeset).toBe(input);
+  });
+
+  it('preserves a non-author attribute already on the insert while adding author', () => {
+    // Insert with a bold attribute but no author.
+    const pool = new AttributePool();
+    const b = pool.putAttrib(['bold', 'true']); // index 0
+    const input = `Z:1>5*${b}+5$Hello`;
+    const {changeset, apool} = stampAuthorOnInserts(input, pool.toJsonable(), AUTHOR);
+    const outPool = (new AttributePool()).fromJsonable(apool);
+    let amap: AttributeMap | null = null;
+    for (const op of deserializeOps(unpack(changeset).ops)) {
+      if (op.opcode === '+') { amap = AttributeMap.fromString(op.attribs, outPool); break; }
+    }
+    expect(amap!.get('author')).toBe(AUTHOR);
+    expect(amap!.get('bold')).toBe('true');
+    expect(() => checkRep(changeset)).not.toThrow();
+  });
+});