[web] recover tracked changes after going offline (#35961)

GitOrigin-RevId: 8d34eb879de91a3b348fc1230eb3ce4d532194e0
2026-11-12 13:13:45 +01:00 · 2026-09-17 11:41:17 +00:00
parent 1f97dbc789
commit 20027166fb
5 changed files with 171 additions and 61 deletions
@@ -107,6 +107,19 @@ export class DocumentContainer extends EventEmitter {
  private chaosMonkeyTimer: number | null = null
  public track_changes_as: string | null = null

+  // Recovered offline edits replay as if they had reached the server before we
+  // went offline, so their tracking state comes from the backup record rather
+  // than the project's current track-changes setting, which may have changed
+  // while we were away. Pinned until the server has the ops, because a
+  // setTrackChangesUserId call arriving from React in between would otherwise
+  // strip meta.tc off the send and the edits would land untracked.
+  private trackChangesPinnedForRecovery = false
+  private deferredTrackChangesUserId: string | null = null
+
+  // Set when a recovery replays tracked edits, cleared once the refetch below
+  // has been kicked off.
+  private needsRangesRefetchAfterRecovery = false
+
  private joinCallbacks: JoinCallback[] = []
  private leaveCallbacks: LeaveCallback[] = []

@@ -238,6 +251,14 @@ export class DocumentContainer extends EventEmitter {
  }

  setTrackChangesUserId(userId: string | null) {
+    if (this.trackChangesPinnedForRecovery) {
+      this.deferredTrackChangesUserId = userId
+      // Hold the editor at the pinned state too, so anything typed during the
+      // window is marked up locally the same way it will be stored server-side:
+      // the whole batch carries meta.tc, whatever the toggle now says.
+      this.cm6?.setTrackChangesUserId(this.track_changes_as)
+      return
+    }
    this.track_changes_as = userId
    if (this.doc) {
      this.doc.setTrackChangesUserId(userId)
@@ -247,6 +268,27 @@ export class DocumentContainer extends EventEmitter {
    }
  }

+  private pinTrackChangesForRecovery(record: OfflineDocBackupRecord) {
+    if (!record.trackChanges) {
+      return
+    }
+    this.deferredTrackChangesUserId = this.track_changes_as
+    this.trackChangesPinnedForRecovery = true
+    this.track_changes_as = getMeta('ol-user_id') ?? 'anonymous'
+    this.doc?.setTrackChangesUserId(this.track_changes_as)
+    this.cm6?.setTrackChangesUserId(this.track_changes_as)
+  }
+
+  private releaseTrackChangesPin() {
+    if (!this.trackChangesPinnedForRecovery) {
+      return
+    }
+    this.trackChangesPinnedForRecovery = false
+    const userId = this.deferredTrackChangesUserId
+    this.deferredTrackChangesUserId = null
+    this.setTrackChangesUserId(userId)
+  }
+
  getTrackingChanges() {
    return !!this.doc?.track_changes
  }
@@ -594,6 +636,8 @@ export class DocumentContainer extends EventEmitter {
  ) {
    const loadWithoutRecovery = (error: unknown) => {
      debugConsole.error('[recovery] failed, loading without recovery', error)
+      this.releaseTrackChangesPin()
+      this.needsRangesRefetchAfterRecovery = false
      const editorContent = this.doc?.getSnapshot() || record.snapshot
      this.ideEventEmitter.emit('ide:unableToSyncOfflineChanges', {
        docId: record.docId,
@@ -620,6 +664,8 @@ export class DocumentContainer extends EventEmitter {
      this.doc.restoreFromOfflineBackup(record)
      this.ranges = new RangesTracker()
      this.bindToShareJsDocEvents()
+      this.pinTrackChangesForRecovery(record)
+      this.needsRangesRefetchAfterRecovery = record.trackChanges
    } catch (error) {
      loadWithoutRecovery(error)
      return
@@ -733,6 +779,7 @@ export class DocumentContainer extends EventEmitter {

    this.detachDoc(this.doc_id, this)

+    this.releaseTrackChangesPin()
    this.offlineBackup?.disconnect()

    this.unBindFromEditorEvents()
@@ -801,11 +848,17 @@ export class DocumentContainer extends EventEmitter {
    )

    this.doc.on('flipped_pending_to_inflight', () => {
+      this.rotateTrackChangesIdSeeds()
      return this.trigger('flipped_pending_to_inflight')
    })

    let docSavedTimeout: number | null
    this.doc.on('saved', () => {
+      this.releaseTrackChangesPin()
+      if (this.needsRangesRefetchAfterRecovery) {
+        this.needsRangesRefetchAfterRecovery = false
+        this.refetchRangesAfterRecovery()
+      }
      if (docSavedTimeout) {
        window.clearTimeout(docSavedTimeout)
      }
@@ -816,6 +869,69 @@ export class DocumentContainer extends EventEmitter {
        this.ideEventEmitter.emit('doc:saved', { doc_id: this.doc_id })
      }, 50)
    })
+
+    this.rotateTrackChangesIdSeeds()
+  }
+
+  // Recovered tracked edits mint their change ids client-side, while the server
+  // mints its own for the same edits, and nothing makes the two agree: the ids
+  // depend on the range state each side had when it applied the ops, and ours is
+  // frozen at the baseline while the server's has moved on. A change whose id we
+  // guessed wrong can't be accepted (the server matches on id and finds
+  // nothing), so once it has our ops we take its ranges as authoritative rather
+  // than trusting the guess. Until this lands, recovered edits may show as plain
+  // text; the text itself is correct throughout.
+  private refetchRangesAfterRecovery() {
+    if (!this.doc) {
+      return
+    }
+    const version = this.doc.getVersion()
+    this.socket.emit(
+      'joinDoc',
+      this.doc_id,
+      version,
+      { encodeRanges: true, supportsHistoryOT: true },
+      (
+        error,
+        docLines,
+        serverVersion,
+        updates,
+        ranges,
+        type = 'sharejs-text-ot'
+      ) => {
+        if (error || type !== 'sharejs-text-ot') {
+          return
+        }
+        // The ranges describe the doc at serverVersion, so they can only be
+        // applied to a doc sitting at that version with nothing outstanding.
+        // Otherwise leave the ids we guessed alone: they may be wrong, but they
+        // are at least positioned against the text we have.
+        if (
+          !this.doc ||
+          this.doc.getVersion() !== serverVersion ||
+          this.doc.hasBufferedOps()
+        ) {
+          return
+        }
+        this.decodeRanges(ranges)
+        this.catchUpRanges(ranges?.changes, ranges?.comments)
+      }
+    )
+  }
+
+  // Tracked-change ids are minted client-side from a seed, and the seed travels
+  // with the batch of ops it belongs to, so the inflight and pending batches
+  // need separate seeds. When the pending batch flips to inflight, the seed our
+  // local edits have been minting from becomes the inflight seed and new edits
+  // start minting from a fresh pending seed.
+  private rotateTrackChangesIdSeeds() {
+    if (!this.doc || !this.ranges) {
+      return
+    }
+    const inflight = this.ranges.getIdSeed()
+    const pending = RangesTracker.generateIdSeed()
+    this.ranges.setIdSeed(pending)
+    this.setTrackChangesIdSeeds({ pending, inflight })
  }

  private onError(
@@ -57,14 +57,7 @@ export class OfflineDocBackup {
    if (!isSplitTestEnabled('intermittent-connection-improvements')) {
      return null
    }
-    const record = OfflineDocBackup.read(projectId, docId)
-    // TODO(35594): tracked-change edits need their track-changes state (user id
-    // + seeds) wired up during recovery to replay as tracked; skip them for now
-    // rather than silently recovering them as untracked edits.
-    if (!record || record.trackChanges) {
-      return null
-    }
-    return record
+    return OfflineDocBackup.read(projectId, docId)
  }

  static remove(projectId: string, docId: string): void {
@@ -180,6 +173,9 @@ export class OfflineDocBackup {
      snapshot: this.baseline.snapshot,
      inflightOp: this.doc.getInflightOp(),
      pendingOp: this.doc.getPendingOp(),
+      // A toggle can't take effect while we are offline: syncTrackChangesState
+      // defers it until the buffer has drained, so this is still the state every
+      // buffered op was made under.
      trackChanges: this.doc.track_changes,
      updatedAt: Date.now(),
      inflightSubmittedIds: Array.from(this.doc.getInflightSubmittedIds()),
@@ -13,7 +13,6 @@ import {
  CommentOperation,
  EditOperation,
 } from '../../../../../types/change'
-import RangesTracker from '@overleaf/ranges-tracker'
 import { rejectChanges } from '@/features/source-editor/extensions/changes/reject-changes'
 import { useCodeMirrorViewContext } from '@/features/source-editor/components/codemirror-context'
 import { postJSON } from '@/infrastructure/fetch-json'
@@ -171,30 +170,6 @@ export const RangesProvider: FC<React.PropsWithChildren> = ({ children }) => {
    }
  }, [currentDocument])

-  // TODO: move this into DocumentContainer?
-  useEffect(() => {
-    if (currentDocument) {
-      const regenerateTrackChangesId = (doc: DocumentContainer) => {
-        if (doc.ranges) {
-          const inflight = doc.ranges.getIdSeed()
-          const pending = RangesTracker.generateIdSeed()
-          doc.ranges.setIdSeed(pending)
-          doc.setTrackChangesIdSeeds({ pending, inflight })
-        }
-      }
-
-      currentDocument.on('flipped_pending_to_inflight', () =>
-        regenerateTrackChangesId(currentDocument)
-      )
-
-      regenerateTrackChangesId(currentDocument)
-
-      return () => {
-        currentDocument.off('flipped_pending_to_inflight')
-      }
-    }
-  }, [currentDocument])
-
  useSocketListener(
    socket,
    'accept-changes',
@@ -263,28 +263,6 @@ describe('EditorManagerProvider docError sync modals', function () {
    cy.findByText('Your offline edits couldn’t be synced').should('not.exist')
  })

-  it('falls back to OutOfSyncModal when the backup holds tracked changes', function () {
-    let key: string
-    cy.then(() => {
-      setSplitTest(true)
-      key = plantBackup(CURRENT_DOC_ID, { trackChanges: true })
-    })
-
-    mount()
-
-    cy.then(() => {
-      currentDoc.trigger('error', new Error('forced'), {}, 'content')
-    })
-
-    cy.findByRole('dialog').should('exist')
-    cy.findByText('Your offline edits couldn’t be synced').should('not.exist')
-
-    // Tracked-change backups are not recovered, so they are left alone.
-    cy.then(() => {
-      expect(window.sessionStorage.getItem(key!)).to.not.equal(null)
-    })
-  })
-
  it('shows UnableToSyncModal on the ide:unableToSyncOfflineChanges recovery event', function () {
    let capturedEmitter: IdeEventEmitter | null = null

@@ -132,9 +132,9 @@ describe('DocumentContainer offline recovery', function () {
    expect(container.getSnapshot()).to.equal('hello')
  })

-  it('skips recovery for tracked-change records', function () {
+  it('replays tracked offline edits as tracked', function () {
    writeRecord({ trackChanges: true })
-    const { container, socket, sent } = makeContainer((fromVersion, cb) => {
+    const { container, sent } = makeContainer((fromVersion, cb) => {
      cb(
        null,
        ['hello'],
@@ -147,10 +147,55 @@ describe('DocumentContainer offline recovery', function () {

    container.join()

-    const calls = joinDocCalls(socket)
-    expect(calls).to.have.length(1)
-    expect(isVersioned(calls[0])).to.equal(false)
-    expect(sent).to.have.length(0)
+    expect(sent).to.have.length(1)
+    // meta.tc is the id seed the server mints its own change ids from, and is
+    // only attached when the batch is tracked.
+    expect(sent[0].meta.tc).to.be.a('string')
+    expect(container.ranges!.changes).to.have.length(1)
+  })
+
+  it('adopts the server ranges once the recovered tracked edits are acked', function () {
+    writeRecord({ trackChanges: true })
+    const serverChange = {
+      id: 'server-minted-id',
+      op: { i: '!', p: 5 },
+      metadata: { user_id: USER_ID },
+    }
+    let joins = 0
+    const { container } = makeContainer((fromVersion, cb) => {
+      joins++
+      if (joins === 1) {
+        cb(
+          null,
+          ['hello'],
+          5,
+          [],
+          { changes: [], comments: [] },
+          'sharejs-text-ot'
+        )
+      } else {
+        cb(
+          null,
+          ['hello!'],
+          5,
+          [],
+          { changes: [serverChange], comments: [] },
+          'sharejs-text-ot'
+        )
+      }
+    })
+
+    container.join()
+    const guessedId = container.ranges!.changes[0].id
+    expect(guessedId).to.not.equal(serverChange.id)
+
+    // the server has stored our op under its own change id
+    container.doc!.clearInflightAndPendingOps()
+    container.doc!.trigger('saved')
+
+    expect(container.ranges!.changes.map(change => change.id)).to.deep.equal([
+      serverChange.id,
+    ])
  })

  it('falls back to a normal load when the server cannot catch us up', function () {