From 08e2a5935afff66ee9fe11ea853f6b3142ef984f Mon Sep 17 00:00:00 2001 From: silver Date: Tue, 25 Aug 2026 13:18:49 +0200 Subject: [PATCH 1/3] fix(setInitialYjsState): do not push or save the initial state on open Opening a document without a stored yjs state applied the generated initial state as a local update. It was then pushed to the server like a user edit, which marked the document dirty and triggered an autosave, even for a freshly created and untouched document. Apply the initial state with the sync provider as origin so it counts as received from the server. The provider sends diffs against the known server state, so the first real user edit still carries the initial state along with it. Signed-off-by: silver Assisted-by: ClaudeCode:claude-fable-5 --- src/components/CollaborativeEditor.vue | 1 + src/helpers/setInitialYjsState.ts | 7 +- src/tests/helpers/setInitialYjsState.spec.ts | 115 +++++++++++++++++++ 3 files changed, 121 insertions(+), 2 deletions(-) create mode 100644 src/tests/helpers/setInitialYjsState.spec.ts diff --git a/src/components/CollaborativeEditor.vue b/src/components/CollaborativeEditor.vue index da0b2f86d22..c2a01092ada 100644 --- a/src/components/CollaborativeEditor.vue +++ b/src/components/CollaborativeEditor.vue @@ -652,6 +652,7 @@ export default defineComponent({ }) setInitialYjsState(this.ydoc, content, { isRichEditor: this.isRichEditor, + origin: this.syncProvider, }) } }) diff --git a/src/helpers/setInitialYjsState.ts b/src/helpers/setInitialYjsState.ts index 543dbe3ddc6..b71928bd144 100644 --- a/src/helpers/setInitialYjsState.ts +++ b/src/helpers/setInitialYjsState.ts @@ -17,11 +17,14 @@ import markdownit from '../markdownit/index.js' * @param content desired content of the final document * @param options options * @param options.isRichEditor use a rich editor for the content + * @param options.origin origin of the update, e.g. the sync provider. + * Pass the sync provider to mark the initial state as received from the server + * so it does not count as local changes that need to be pushed and saved. */ export function setInitialYjsState( ydoc: Doc, content: string, - { isRichEditor }: { isRichEditor: boolean }, + { isRichEditor, origin }: { isRichEditor: boolean, origin?: unknown }, ) { const html = isRichEditor ? markdownit.render(content) + '

' @@ -54,5 +57,5 @@ export function setInitialYjsState( } const baseUpdate = encodeStateAsUpdate(getBaseDoc(node)) - applyUpdate(ydoc, baseUpdate) + applyUpdate(ydoc, baseUpdate, origin) } diff --git a/src/tests/helpers/setInitialYjsState.spec.ts b/src/tests/helpers/setInitialYjsState.spec.ts new file mode 100644 index 00000000000..8a45907784f --- /dev/null +++ b/src/tests/helpers/setInitialYjsState.spec.ts @@ -0,0 +1,115 @@ +/** + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +import type { Mock } from 'vitest' + +import * as decoding from 'lib0/decoding' +import * as encoding from 'lib0/encoding' +import mitt from 'mitt' +import { describe, expect, it, vi } from 'vitest' +import * as syncProtocol from 'y-protocols/sync' +import * as Y from 'yjs' +import { encodeArrayBuffer } from '../../helpers/base64.ts' +import { setInitialYjsState } from '../../helpers/setInitialYjsState.ts' +import initWebSocketPolyfill from '../../services/WebSocketPolyfill.ts' +import { messageSync, WebsocketProvider } from '../../services/y-websocket.js' + +describe('setInitialYjsState', () => { + it('applies the content to the ydoc', () => { + const ydoc = new Y.Doc() + setInitialYjsState(ydoc, '# Hello world', { isRichEditor: true }) + expect(ydoc.getXmlFragment('default').length).toBeGreaterThan(0) + }) + + it('passes the given origin to the update', () => { + const ydoc = new Y.Doc() + const origin = { iAmTheOrigin: true } + const updateHandler = vi.fn() + ydoc.on('update', updateHandler) + setInitialYjsState(ydoc, '# Hello world', { isRichEditor: true, origin }) + expect(updateHandler).toHaveBeenCalledTimes(1) + expect(updateHandler.mock.calls[0][1]).toBe(origin) + }) + + describe('with the sync provider as origin', () => { + // Sync update messages sort between 'AAE' and 'AQ' in base64, + // matching the classification in Outbox.storeStep. + const isSyncUpdate = (step: Uint8Array) => { + const encoded = encodeArrayBuffer(step) + return encoded >= 'AAE' && encoded < 'AQ' + } + + const setupProvider = async (ydoc: Y.Doc) => { + const syncService = { + bus: mitt(), + open: vi.fn(async () => ({})), + hasActiveConnection: vi.fn(() => true), + sendStep: vi.fn(), + version: -1, + } + const WebSocketPolyfill = initWebSocketPolyfill( + syncService as any, + 123, + ) + const provider = new WebsocketProvider( + 'ws://localhost:1234', + 'file:123', + ydoc, + { WebSocketPolyfill: WebSocketPolyfill as any, disableBc: true }, + ) + // wait for the deferred onopen call of the polyfill + await vi.waitUntil(() => provider.wsconnected) + const sentSyncUpdates = () => (syncService.sendStep as Mock).mock.calls + .map(([step]) => step) + .filter(isSyncUpdate) + return { provider, sentSyncUpdates } + } + + it('does not send the initial state as a step', async () => { + const ydoc = new Y.Doc() + const { provider, sentSyncUpdates } = await setupProvider(ydoc) + setInitialYjsState(ydoc, '# Hello world', { + isRichEditor: true, + origin: provider, + }) + expect(sentSyncUpdates()).toHaveLength(0) + }) + + it('sends the initial state along with later local changes', async () => { + const ydoc = new Y.Doc() + const { provider, sentSyncUpdates } = await setupProvider(ydoc) + setInitialYjsState(ydoc, '# Hello world', { + isRichEditor: true, + origin: provider, + }) + // a local change without origin - as caused by user edits + const paragraph = new Y.XmlElement('paragraph') + paragraph.insert(0, [new Y.XmlText('typed later')]) + ydoc.getXmlFragment('default').insert(0, [paragraph]) + const sent = sentSyncUpdates() + expect(sent).toHaveLength(1) + // the sent update also contains the initial state + const receiving = new Y.Doc() + const decoder = decoding.createDecoder(sent[0]) + expect(decoding.readVarUint(decoder)).toBe(messageSync) + syncProtocol.readSyncMessage( + decoder, + encoding.createEncoder(), + receiving, + 'test', + ) + const received = receiving.getXmlFragment('default').toJSON() + expect(received).toContain('Hello world') + expect(received).toContain('typed later') + }) + + it('sends the initial state as a step without an origin', async () => { + const ydoc = new Y.Doc() + const { sentSyncUpdates } = await setupProvider(ydoc) + setInitialYjsState(ydoc, '# Hello world', { isRichEditor: true }) + expect(sentSyncUpdates()).toHaveLength(1) + }) + }) +}) From 9dcbbde71197481ad75c5abb2f96d85041649102 Mon Sep 17 00:00:00 2001 From: silver Date: Mon, 31 Aug 2026 16:29:44 +0200 Subject: [PATCH 2/3] fix(setInitialYjsState): revert first commit Signed-off-by: silver Assisted-by: ClaudeCode:claude-sonnet-5 --- src/components/CollaborativeEditor.vue | 5 +- src/helpers/setInitialYjsState.ts | 7 +- src/tests/helpers/setInitialYjsState.spec.ts | 115 ------------------- 3 files changed, 6 insertions(+), 121 deletions(-) delete mode 100644 src/tests/helpers/setInitialYjsState.spec.ts diff --git a/src/components/CollaborativeEditor.vue b/src/components/CollaborativeEditor.vue index c2a01092ada..321a068ef4c 100644 --- a/src/components/CollaborativeEditor.vue +++ b/src/components/CollaborativeEditor.vue @@ -650,9 +650,12 @@ export default defineComponent({ content, isRichEditor: this.isRichEditor, }) + // The resulting push still needs to reach the server like any + // other step, but should not autosave a document nobody has + // edited yet. + this.saveService.skipNextAutosaveTrigger() setInitialYjsState(this.ydoc, content, { isRichEditor: this.isRichEditor, - origin: this.syncProvider, }) } }) diff --git a/src/helpers/setInitialYjsState.ts b/src/helpers/setInitialYjsState.ts index b71928bd144..543dbe3ddc6 100644 --- a/src/helpers/setInitialYjsState.ts +++ b/src/helpers/setInitialYjsState.ts @@ -17,14 +17,11 @@ import markdownit from '../markdownit/index.js' * @param content desired content of the final document * @param options options * @param options.isRichEditor use a rich editor for the content - * @param options.origin origin of the update, e.g. the sync provider. - * Pass the sync provider to mark the initial state as received from the server - * so it does not count as local changes that need to be pushed and saved. */ export function setInitialYjsState( ydoc: Doc, content: string, - { isRichEditor, origin }: { isRichEditor: boolean, origin?: unknown }, + { isRichEditor }: { isRichEditor: boolean }, ) { const html = isRichEditor ? markdownit.render(content) + '

' @@ -57,5 +54,5 @@ export function setInitialYjsState( } const baseUpdate = encodeStateAsUpdate(getBaseDoc(node)) - applyUpdate(ydoc, baseUpdate, origin) + applyUpdate(ydoc, baseUpdate) } diff --git a/src/tests/helpers/setInitialYjsState.spec.ts b/src/tests/helpers/setInitialYjsState.spec.ts deleted file mode 100644 index 8a45907784f..00000000000 --- a/src/tests/helpers/setInitialYjsState.spec.ts +++ /dev/null @@ -1,115 +0,0 @@ -/** - * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors - * SPDX-License-Identifier: AGPL-3.0-or-later - */ - -import type { Mock } from 'vitest' - -import * as decoding from 'lib0/decoding' -import * as encoding from 'lib0/encoding' -import mitt from 'mitt' -import { describe, expect, it, vi } from 'vitest' -import * as syncProtocol from 'y-protocols/sync' -import * as Y from 'yjs' -import { encodeArrayBuffer } from '../../helpers/base64.ts' -import { setInitialYjsState } from '../../helpers/setInitialYjsState.ts' -import initWebSocketPolyfill from '../../services/WebSocketPolyfill.ts' -import { messageSync, WebsocketProvider } from '../../services/y-websocket.js' - -describe('setInitialYjsState', () => { - it('applies the content to the ydoc', () => { - const ydoc = new Y.Doc() - setInitialYjsState(ydoc, '# Hello world', { isRichEditor: true }) - expect(ydoc.getXmlFragment('default').length).toBeGreaterThan(0) - }) - - it('passes the given origin to the update', () => { - const ydoc = new Y.Doc() - const origin = { iAmTheOrigin: true } - const updateHandler = vi.fn() - ydoc.on('update', updateHandler) - setInitialYjsState(ydoc, '# Hello world', { isRichEditor: true, origin }) - expect(updateHandler).toHaveBeenCalledTimes(1) - expect(updateHandler.mock.calls[0][1]).toBe(origin) - }) - - describe('with the sync provider as origin', () => { - // Sync update messages sort between 'AAE' and 'AQ' in base64, - // matching the classification in Outbox.storeStep. - const isSyncUpdate = (step: Uint8Array) => { - const encoded = encodeArrayBuffer(step) - return encoded >= 'AAE' && encoded < 'AQ' - } - - const setupProvider = async (ydoc: Y.Doc) => { - const syncService = { - bus: mitt(), - open: vi.fn(async () => ({})), - hasActiveConnection: vi.fn(() => true), - sendStep: vi.fn(), - version: -1, - } - const WebSocketPolyfill = initWebSocketPolyfill( - syncService as any, - 123, - ) - const provider = new WebsocketProvider( - 'ws://localhost:1234', - 'file:123', - ydoc, - { WebSocketPolyfill: WebSocketPolyfill as any, disableBc: true }, - ) - // wait for the deferred onopen call of the polyfill - await vi.waitUntil(() => provider.wsconnected) - const sentSyncUpdates = () => (syncService.sendStep as Mock).mock.calls - .map(([step]) => step) - .filter(isSyncUpdate) - return { provider, sentSyncUpdates } - } - - it('does not send the initial state as a step', async () => { - const ydoc = new Y.Doc() - const { provider, sentSyncUpdates } = await setupProvider(ydoc) - setInitialYjsState(ydoc, '# Hello world', { - isRichEditor: true, - origin: provider, - }) - expect(sentSyncUpdates()).toHaveLength(0) - }) - - it('sends the initial state along with later local changes', async () => { - const ydoc = new Y.Doc() - const { provider, sentSyncUpdates } = await setupProvider(ydoc) - setInitialYjsState(ydoc, '# Hello world', { - isRichEditor: true, - origin: provider, - }) - // a local change without origin - as caused by user edits - const paragraph = new Y.XmlElement('paragraph') - paragraph.insert(0, [new Y.XmlText('typed later')]) - ydoc.getXmlFragment('default').insert(0, [paragraph]) - const sent = sentSyncUpdates() - expect(sent).toHaveLength(1) - // the sent update also contains the initial state - const receiving = new Y.Doc() - const decoder = decoding.createDecoder(sent[0]) - expect(decoding.readVarUint(decoder)).toBe(messageSync) - syncProtocol.readSyncMessage( - decoder, - encoding.createEncoder(), - receiving, - 'test', - ) - const received = receiving.getXmlFragment('default').toJSON() - expect(received).toContain('Hello world') - expect(received).toContain('typed later') - }) - - it('sends the initial state as a step without an origin', async () => { - const ydoc = new Y.Doc() - const { sentSyncUpdates } = await setupProvider(ydoc) - setInitialYjsState(ydoc, '# Hello world', { isRichEditor: true }) - expect(sentSyncUpdates()).toHaveLength(1) - }) - }) -}) From 44233fa67b6e6222f59b1e73a35483c1e7143a50 Mon Sep 17 00:00:00 2001 From: silver Date: Mon, 31 Aug 2026 16:40:43 +0200 Subject: [PATCH 3/3] fix(autosave): do not autosave the generated initial document state Opening a document without a stored yjs state applies a generated initial state to the ydoc. That push still needs to reach the server and be recorded as a normal step, so recovery and later clients can rely on the full step history. But it should not, on its own, trigger an autosave of a document the has not edited yet. Arm a one-shot skip on SaveService before generating the initial state, consumed by the very next changesPushed-triggered autosave attempt. Signed-off-by: silver Assisted-by: ClaudeCode:claude-sonnet-5 --- src/composables/useSaveService.ts | 4 +- src/services/SaveService.ts | 15 +++++++ src/tests/services/SaveService.spec.ts | 60 ++++++++++++++++++++++++++ 3 files changed, 77 insertions(+), 2 deletions(-) create mode 100644 src/tests/services/SaveService.spec.ts diff --git a/src/composables/useSaveService.ts b/src/composables/useSaveService.ts index 7d1b3bfa9d0..0b14de6cc53 100644 --- a/src/composables/useSaveService.ts +++ b/src/composables/useSaveService.ts @@ -49,10 +49,10 @@ export function provideSaveService( getSaveData, }) - syncService.bus.on('changesPushed', saveService.autosave) + syncService.bus.on('changesPushed', saveService.autosaveOnChangesPushed) syncService.bus.on('close', saveService.clear) onUnmounted(() => { - syncService.bus.off('changesPushed', saveService.autosave) + syncService.bus.off('changesPushed', saveService.autosaveOnChangesPushed) syncService.bus.off('close', saveService.clear) }) diff --git a/src/services/SaveService.ts b/src/services/SaveService.ts index 0992ea12d71..2a1aa66b22d 100644 --- a/src/services/SaveService.ts +++ b/src/services/SaveService.ts @@ -39,7 +39,9 @@ class SaveService { pendingAutosave = 0 getSaveData autosave + autosaveOnChangesPushed clear + #skipNextAutosaveTrigger = false constructor({ connection, @@ -54,9 +56,22 @@ class SaveService { this.document = document this.getSaveData = getSaveData this.autosave = debounce(this._autosave.bind(this), AUTOSAVE_DEBOUNCE * 1000) + this.autosaveOnChangesPushed = this._autosaveOnChangesPushed.bind(this) this.clear = this.clearAutosave.bind(this) } + skipNextAutosaveTrigger() { + this.#skipNextAutosaveTrigger = true + } + + _autosaveOnChangesPushed() { + if (this.#skipNextAutosaveTrigger) { + this.#skipNextAutosaveTrigger = false + return + } + this.autosave() + } + /** * Save the current state * diff --git a/src/tests/services/SaveService.spec.ts b/src/tests/services/SaveService.spec.ts new file mode 100644 index 00000000000..f2bf08e6a68 --- /dev/null +++ b/src/tests/services/SaveService.spec.ts @@ -0,0 +1,60 @@ +/** + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +import { describe, expect, it, vi } from 'vitest' +import { shallowRef } from 'vue' +import { SaveService } from '../../services/SaveService.ts' + +function createSaveService() { + return new SaveService({ + connection: shallowRef(undefined), + document: shallowRef(undefined), + getSaveData: vi.fn(), + }) +} + +describe('SaveService.autosaveOnChangesPushed', () => { + it('autosaves for a normal changesPushed trigger', () => { + const saveService = createSaveService() + const autosave = vi.spyOn(saveService, 'autosave') + + saveService.autosaveOnChangesPushed() + + expect(autosave).toHaveBeenCalledOnce() + }) + + it('does not autosave right after skipNextAutosaveTrigger', () => { + const saveService = createSaveService() + const autosave = vi.spyOn(saveService, 'autosave') + + saveService.skipNextAutosaveTrigger() + saveService.autosaveOnChangesPushed() + + expect(autosave).not.toHaveBeenCalled() + }) + + it('only skips once - the next trigger autosaves normally', () => { + const saveService = createSaveService() + const autosave = vi.spyOn(saveService, 'autosave') + + saveService.skipNextAutosaveTrigger() + saveService.autosaveOnChangesPushed() + saveService.autosaveOnChangesPushed() + + expect(autosave).toHaveBeenCalledOnce() + }) + + it('is unaffected by unrelated triggers before the skip is armed', () => { + const saveService = createSaveService() + const autosave = vi.spyOn(saveService, 'autosave') + + saveService.autosaveOnChangesPushed() + saveService.skipNextAutosaveTrigger() + saveService.autosaveOnChangesPushed() + saveService.autosaveOnChangesPushed() + + expect(autosave).toHaveBeenCalledTimes(2) + }) +})