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

* fix(editor): tag inserts with clientVars author until userAuthor propagates

Fixes the intermittent Firefox `clear_authorship_color` flake ("clear authorship
colors can be undone to restore author colors").

Root cause: the inner editor's `thisAuthor` starts '' and is only populated when
collab_client's `setProperty('userAuthor', userId)` reaches the inner frame —
that call is queued by the outer ace wrapper via pendingInit until the iframe
loads, so it is applied asynchronously. Under Firefox timing the first keystrokes
can beat it, so freshly typed text is tagged author=''. An empty author
canonicalizes to no-author, producing an unattributed insert (`Z:1>5+5$Hello`)
that the server's pad-corruption guard rejects ("submitted an insert without an
author attribute"), dropping the whole USER_CHANGES and losing authorship. Undo
then cannot restore an author color and the test sees `<span class="">`.

Fix: a `getLocalAuthor()` helper that falls back to `clientVars.userId` (the same
author id, available synchronously in the inner frame — already used as `myId`
elsewhere) whenever `thisAuthor` is still empty, applied at the three new-text
insert sites. The intentional clear-authorship path (`['author','']`) and the
server-side guard are unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(editor): also seed line-attribute author to close the same race

The first commit fixed text inserts, but line-attribute changes (lists, headings,
alignment) emit a line-marker insert tagged with AttributeManager.author, which
likewise starts '' and is only set when the async setProperty('userAuthor')
lands. An early list/heading could therefore emit an unattributed line-marker
insert and hit the same server-side rejection.

Seed documentAttributeManager.author from getLocalAuthor() (clientVars fallback)
right after it is constructed; the userauthor handler keeps it in sync after.

Audited the other author-tagging paths: all ace2_inner text-insert sites go
through getLocalAuthor()/authorizer now, and contentcollector (paste/import)
derives authorship from the pasted DOM's author- classes via className2Author —
a different mechanism, not this async race — and self-guards on `if (state.author)`.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(editor): read the local author from the TOP pad window, not the inner frame

v1 of this fix fell back to `window.clientVars?.userId`, but that did NOT resolve
the flake — CI reproduced the identical failure and the server log still showed
the unattributed-insert rejection.

Ground truth via direct DOM measurement of a running pad: the inner editor
iframe's `window.clientVars` is NEVER populated (undefined at t=0/1/3/6s for the
life of the pad), so the v1 fallback always returned ''. The author id lives on
the TOP pad window (`window.top.clientVars.userId`), set by pad.ts when the
CLIENT_VARS message arrives — which necessarily precedes editor creation, so it
is reliably available from the inner frame the moment the editor can accept
input.

getLocalAuthor() now reads window.top.clientVars.userId (guarded with try/catch
for cross-origin embedded pads). thisAuthor still takes precedence once the
queued setProperty('userAuthor') lands. All text-insert sites and the
line-attribute seed already route through getLocalAuthor(), so both paths are
covered.

Verification: ts-check passes; fix source confirmed by direct measurement (the
top window reliably holds the author at editor-init time). Local end-to-end
repro was not possible — the dev server's require-kernel bundle did not reflect
working-tree edits — so this is validated against the measured source + CI.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(editor): walk ancestor frames for the author instead of window.top

Addresses a correctness gap flagged in review: reading window.top.clientVars
fails for same-origin embeds, where window.top is the host page (accessible, so
no exception is thrown) and has no Etherpad clientVars — getLocalAuthor() then
returns '' and the unattributed-insert bug recurs.

Walk up the ancestor chain (inner -> ace_outer -> pad window) and return the
first frame that has clientVars.userId. This stops at the pad window and never
depends on window.top, so it is correct for normal pads, same-origin embeds, and
cross-origin embeds (the try/catch ends the walk if an ancestor is cross-origin,
though the pad window — always same-origin with the editor frames — is reached
first). Behavior in the non-embed case is unchanged (pad window is 2 hops up).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(collab): stamp the author onto outgoing inserts in collab_client

Replaces the earlier getLocalAuthor attempts (v1-v3, reverted from ace2_inner.ts),
which all failed because they tried to *read* an author that genuinely does not
exist yet: the local author id (clientVars.userId) only arrives with the
CLIENT_VARS socket message, and under load (Firefox + plugins) the editor can
become editable and the user can type before that message lands. At that instant
there is no author anywhere client-side, so the editor emits an unattributed
insert that the server's pad-corruption guard rejects — dropping the change and
losing authorship (the clear_authorship_color flake).

Fix where the author IS reliably available: collab_client only exists after
CLIENT_VARS has arrived, so its `userId` is always populated. A new pure helper
stampAuthorOnInserts() rewrites the outgoing wire changeset so every '+' op
carries an author, stamping userId onto any that lack one, just before it goes on
the wire (both the USER_CHANGES send and the reconnect/further-changeset path).
Independent of editor-init timing.

Verification:
- Unit test (stampAuthorOnInserts.test.ts, 5 cases): stamps the exact flake
  changeset `Z:1>5+5$Hello` -> authored + checkRep-valid + text preserved; leaves
  already-attributed inserts and keep/remove-only changesets unchanged; never
  invents an author when userId is empty; preserves other attributes.
- ace2_inner.ts reverted to develop (the editor is unchanged; the fix is fully
  localized to the send path).
- Local run of the clear_authorship_color spec with the fix: all pass, no
  server-side unattributed-insert rejections; full backend-new vitest green
  (the unrelated backend-tests-glob env-only failure aside).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

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();
+  });
+});