From 20027166fbefc1abc912a21b0dfa8e32e1e86a28 Mon Sep 17 00:00:00 2001 From: Kristina <7614497+khjrtbrg@users.noreply.github.com> Date: Thu, 30 Jul 2026 09:17:23 +0200 Subject: [PATCH] [web] recover tracked changes after going offline (#35961) GitOrigin-RevId: 8d34eb879de91a3b348fc1230eb3ce4d532194e0 --- .../ide-react/editor/document-container.ts | 116 ++++++++++++++++++ .../ide-react/editor/offline-doc-backup.ts | 12 +- .../review-panel/context/ranges-context.tsx | 25 ---- ...ditor-manager-context-out-of-sync.spec.tsx | 22 ---- .../document-container-recovery.test.ts | 57 ++++++++- 5 files changed, 171 insertions(+), 61 deletions(-) diff --git a/services/web/frontend/js/features/ide-react/editor/document-container.ts b/services/web/frontend/js/features/ide-react/editor/document-container.ts index f864186726..c2feaa3dae 100644 --- a/services/web/frontend/js/features/ide-react/editor/document-container.ts +++ b/services/web/frontend/js/features/ide-react/editor/document-container.ts @@ -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( diff --git a/services/web/frontend/js/features/ide-react/editor/offline-doc-backup.ts b/services/web/frontend/js/features/ide-react/editor/offline-doc-backup.ts index 67ed2b2fb7..e687870375 100644 --- a/services/web/frontend/js/features/ide-react/editor/offline-doc-backup.ts +++ b/services/web/frontend/js/features/ide-react/editor/offline-doc-backup.ts @@ -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()), diff --git a/services/web/frontend/js/features/review-panel/context/ranges-context.tsx b/services/web/frontend/js/features/review-panel/context/ranges-context.tsx index dba1de57e5..8868a35bd4 100644 --- a/services/web/frontend/js/features/review-panel/context/ranges-context.tsx +++ b/services/web/frontend/js/features/review-panel/context/ranges-context.tsx @@ -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 = ({ 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', diff --git a/services/web/test/frontend/features/ide-react/context/editor-manager-context-out-of-sync.spec.tsx b/services/web/test/frontend/features/ide-react/context/editor-manager-context-out-of-sync.spec.tsx index 7c87f73c66..38136351f9 100644 --- a/services/web/test/frontend/features/ide-react/context/editor-manager-context-out-of-sync.spec.tsx +++ b/services/web/test/frontend/features/ide-react/context/editor-manager-context-out-of-sync.spec.tsx @@ -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 diff --git a/services/web/test/frontend/features/ide-react/unit/editor/document-container-recovery.test.ts b/services/web/test/frontend/features/ide-react/unit/editor/document-container-recovery.test.ts index 56a52575c3..44cc483c67 100644 --- a/services/web/test/frontend/features/ide-react/unit/editor/document-container-recovery.test.ts +++ b/services/web/test/frontend/features/ide-react/unit/editor/document-container-recovery.test.ts @@ -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 () {