From d2ec173a1bd8d54bc2a5d88fd637ec89bd9df206 Mon Sep 17 00:00:00 2001 From: Jannik Stehle Date: Mon, 7 Sep 2026 14:50:04 +0200 Subject: [PATCH 1/2] fix(yjs): let the server decide who seeds an empty room Two clients opening the same file at once could both seed the Y.Doc, and the room then held the body twice. The awareness election that guarded this only waited a guessed 150ms, and a client has no signal that its announce is visible to everyone. The Yjs server now grants seeding to exactly one connection per room. It is the only party that sees every connection at once, so no wait has to be guessed. The grant is permission, not an instruction: the client still checks its own document first, so a grant for a room that already has content costs nothing. It passes to another writer when the holder leaves, and dies with the room. --- .../src/composables/yjs/useYjsSession.ts | 211 ++++++---- .../composables/yjs/useYjsSession.spec.ts | 369 ++++++++++++++++-- services/yjs/README.md | 12 + services/yjs/src/lib/hooks.ts | 67 +++- services/yjs/src/lib/seedGrant.ts | 67 ++++ services/yjs/tests/unit/lib/hooks.spec.ts | 173 +++++++- .../features/yjs/collaborativeEditing.feature | 8 +- tests/e2e/features/yjs/hydration.feature | 68 ++++ tests/e2e/steps/ui/resources.ts | 57 ++- .../support/objects/app-files/utils/editor.ts | 3 + 10 files changed, 886 insertions(+), 149 deletions(-) create mode 100644 services/yjs/src/lib/seedGrant.ts create mode 100644 tests/e2e/features/yjs/hydration.feature diff --git a/packages/web-pkg/src/composables/yjs/useYjsSession.ts b/packages/web-pkg/src/composables/yjs/useYjsSession.ts index 37314d5e572..d5945335f65 100644 --- a/packages/web-pkg/src/composables/yjs/useYjsSession.ts +++ b/packages/web-pkg/src/composables/yjs/useYjsSession.ts @@ -71,9 +71,10 @@ export interface YjsSession { provider: ShallowRef status: ShallowRef /** - * False until initial sync and the hydration decision have settled. - * Consumers gate the editor mount on this to avoid a brief empty-editor - * flash while hydration runs. + * False until initial sync and the hydration decision have settled. In + * remote mode that decision is a round trip to the Yjs server. Consumers + * gate the editor mount on this to avoid a brief empty-editor flash while + * hydration runs. */ isReady: ShallowRef /** @@ -129,29 +130,28 @@ const CONNECT_TIMEOUT_MS = 10_000 const LOCAL_SAVE_ORIGIN = 'local-save' const STALE_RECOVERY_RESET_ORIGIN = 'stale-recovery-reset' const STALE_RECOVERY_COMMIT_ORIGIN = 'stale-recovery-commit' +const HYDRATE_ORIGIN = 'hydrate' const OWN_META_ORIGINS: unknown[] = [ LOCAL_SAVE_ORIGIN, STALE_RECOVERY_RESET_ORIGIN, - STALE_RECOVERY_COMMIT_ORIGIN + STALE_RECOVERY_COMMIT_ORIGIN, + HYDRATE_ORIGIN ] -/** - * Awareness field: whether this client is able to seed an empty room. - * Read-only clients set it false, so the hydration election can skip them - - * the Yjs server rejects their writes, so their "win" would leave the room - * empty for everyone. - */ -const SEED_CAPABLE_KEY = '_oc_canSeed' /** * How long `wasWrittenByRoom` waits for a peer's etag stamp that may still be * in flight at conflict time. Only ever paid in full for a genuine external * write while peers are present. */ const ROOM_ETAG_GRACE_MS = 1_000 +/** + * Stateless messages that arbitrate who seeds an empty room. Must match + * `SeedMessage` in `services/yjs/src/lib/seedGrant.ts`, keep the two in sync. + */ +const SEED_REQUEST = '_oc_seed_request' +const SEED_GRANTED = '_oc_seed_granted' +const SEED_DENIED = '_oc_seed_denied' const FALLBACK_WEB_VERSION = '0.0.0' -/** The awareness fields this composable sets or reads. */ -type AwarenessState = Record & { [SEED_CAPABLE_KEY]?: boolean } - function resolveWebVersion(): string { const version = process.env.PACKAGE_VERSION?.trim() return version || FALLBACK_WEB_VERSION @@ -486,7 +486,8 @@ export function useYjsSession(options: YjsSessionOptions): YjsSession { return value } - // True while recovery rewrites the doc. See `canReportContent`. + // True while the session itself writes the live doc (recovery, late seed). + // See `canReportContent`. let isRewritingDoc = false /** @@ -505,15 +506,108 @@ export function useYjsSession(options: YjsSessionOptions): YjsSession { } /** - * Hydration: the elected client seeds the Y.Doc from native content, lowest - * awareness clientId wins. In local mode there are no peers, so the - * election degenerates to "we win unconditionally". + * The seed request this session is waiting on, if any. Also the marker for + * "a hydration is in flight": the request is the only thing hydration ever + * awaits. */ - async function runInitialHydration( - doc: Y.Doc, - prov: HocuspocusProvider | null, - awarenessInstance: Awareness - ) { + let pendingSeedGrant: ((granted: boolean) => void) | null = null + + /** + * Abandon a request whose session is gone. Answered as a refusal so the + * hydration it belongs to unwinds without touching the new session's doc. + */ + function abandonSeedGrant() { + pendingSeedGrant?.(false) + } + + /** Ask the Yjs server whether we may seed this room. */ + function requestSeedGrant(prov: HocuspocusProvider): Promise { + return new Promise((resolve) => { + pendingSeedGrant = (granted) => { + pendingSeedGrant = null + resolve(granted) + } + prov.sendStateless(SEED_REQUEST) + }) + } + + /** + * The server's answer to a seed request, or an unsolicited grant when the + * previous holder left the room without seeding it. + */ + function onSeedMessage(doc: Y.Doc, prov: HocuspocusProvider, payload: string) { + if (payload !== SEED_GRANTED && payload !== SEED_DENIED) return + const granted = payload === SEED_GRANTED + if (pendingSeedGrant) { + pendingSeedGrant(granted) + return + } + if (granted) seedOnUnsolicitedGrant(doc, prov) + } + + /** + * Write the initial body, and announce it so a read-only peer can drop the + * private copy it hydrated while the room was still empty. + */ + function seedDoc(doc: Y.Doc, current: YjsAdapter, meta: SessionMetaMap) { + try { + // Announce and body in one transaction, so a read-only peer sees the + // announce and the body together. Its meta observer then rebuilds the + // session from the room's state and cancels the pending report, so the + // merged private copy is never shown for long or reported. + doc.transact(() => { + meta.set('hydrated', true) + current.hydrate(doc, toValue(currentContent)) + }, HYDRATE_ORIGIN) + } catch (e) { + // Withdraw the announce, or a read-only peer waits forever for a body + // that is never coming. + doc.transact(() => meta.delete('hydrated'), HYDRATE_ORIGIN) + throw e + } + } + + /** Hydration threw. Lock rather than mount an editable empty doc. */ + function failHydration(prov: HocuspocusProvider | null, e: unknown) { + console.error('[yjs] hydration failed:', e) + lockForReload( + prov, + $gettext('Preparing this file for collaborative editing failed. Please reload.') + ) + } + + /** + * A grant that arrived on its own, because the previous holder left the room + * without seeding it. The grant is permission, not an instruction, so every + * reason not to seed is re-checked here. + */ + function seedOnUnsolicitedGrant(doc: Y.Doc, prov: HocuspocusProvider) { + if (doc.isDestroyed || unref(ydoc) !== doc) return + // Not synced yet: the room's content may still be on the way. Hydration + // asks for its own grant once it runs, and the server answers the holder + // with another grant. + if (!unref(isReady)) return + if (unref(effectiveReadOnly)) return + const current = toValue(adapter) + if (current.hasContent(doc)) return + // The session is live, so the seed must not be reported as an edit. + isRewritingDoc = true + try { + seedDoc(doc, current, sessionMeta(doc)) + lastReportedStateVector = Y.encodeStateVector(doc) + } catch (e) { + failHydration(prov, e) + } finally { + isRewritingDoc = false + } + } + + /** + * Hydration: one client per room seeds the Y.Doc from native content, and + * the Yjs server picks which - see `requestSeedGrant`. In local mode there + * is no room, so no permission is needed. + */ + async function runInitialHydration(doc: Y.Doc, prov: HocuspocusProvider | null) { const current = toValue(adapter) const meta = sessionMeta(doc) @@ -550,6 +644,10 @@ export function useYjsSession(options: YjsSessionOptions): YjsSession { if (current.hasContent(doc)) return + // Seeding is a first-entry decision. Mid-session this is a reconnect, and + // an empty doc here is one the user emptied - the room takes it as is. + if (unref(isReady)) return + // Read-only client in an empty room: hydrate a private copy so the file // is not shown blank. It never reaches the room (the server rejects // read-only writes); `hasLocalOnlyContent` lets the meta observer drop it @@ -563,37 +661,14 @@ export function useYjsSession(options: YjsSessionOptions): YjsSession { return } - // Election to avoid double-hydration: let peers announce themselves via - // awareness, then the lowest seed-capable clientId wins. Skipped in local - // mode, where the announce wait would only delay first paint. + // Let the server pick who seeds. Skipped in local mode. if (prov) { - await new Promise((resolve) => setTimeout(resolve, 150)) - - if (current.hasContent(doc)) return // someone beat us - - // A peer announced its seeding but its content has not landed yet. - // `hasContent` is still false at that point. - if (meta.get('hydrated') === true) return - - // A missing flag counts as seed-capable. - const myId = doc.clientID - const peers = Array.from(awarenessInstance.getStates().entries()) - .filter(([, state]) => (state as AwarenessState)?.[SEED_CAPABLE_KEY] !== false) - .map(([clientId]) => clientId) - const lowest = peers.length ? Math.min(myId, ...peers) : myId - if (myId !== lowest) return + if (!(await requestSeedGrant(prov))) return + // Content may have landed while we waited. + if (current.hasContent(doc)) return } - // Announce before seeding, so read-only peers drop their private copy - // before our content lands rather than merge with it. - doc.transact(() => meta.set('hydrated', true)) - try { - current.hydrate(doc, toValue(currentContent)) - } catch (e) { - // Withdraw the announce so the next joiner can seed for real. - doc.transact(() => meta.delete('hydrated')) - throw e - } + seedDoc(doc, current, meta) } /** @@ -677,21 +752,13 @@ export function useYjsSession(options: YjsSessionOptions): YjsSession { * `ydoc.value === doc` guard keeps a stale invocation from clearing the * loading state of the next session. */ - async function onProviderSynced( - doc: Y.Doc, - prov: HocuspocusProvider | null, - awarenessInstance: Awareness - ) { + async function onProviderSynced(doc: Y.Doc, prov: HocuspocusProvider | null) { try { - await runInitialHydration(doc, prov, awarenessInstance) + await runInitialHydration(doc, prov) } catch (e) { // Call sites fire this without awaiting, so an escaping rejection would // leave a half-hydrated document with no explanation. - console.error('[yjs] hydration failed:', e) - lockForReload( - prov, - $gettext('Preparing this file for collaborative editing failed. Please reload.') - ) + failHydration(prov, e) } finally { if (!doc.isDestroyed && unref(ydoc) === doc) { // Baseline for a save before any edit: the hydrated doc is the same @@ -784,11 +851,21 @@ export function useYjsSession(options: YjsSessionOptions): YjsSession { // populated; re-running the hydration checks there could plant a // stale flag against a room this client has just left. if (unref(isReady)) return - void onProviderSynced(doc, null, prov.awareness!) + void onProviderSynced(doc, null) + }, + onStateless({ payload }) { + onSeedMessage(doc, prov, payload) }, onSynced() { clearConnectTimer() - void onProviderSynced(doc, prov, prov.awareness!) + // Reconnected inside the seed wait. The answer to the old socket is + // lost, so ask again on this one instead of starting a second + // hydration next to the one still waiting. + if (pendingSeedGrant) { + prov.sendStateless(SEED_REQUEST) + return + } + void onProviderSynced(doc, prov) } }) @@ -811,14 +888,13 @@ export function useYjsSession(options: YjsSessionOptions): YjsSession { // into a room that may already hold the same content. stopProvider(prov) // `runInitialHydration` bails if content synced in after all. - void onProviderSynced(doc, null, prov.awareness!) + void onProviderSynced(doc, null) }, CONNECT_TIMEOUT_MS) // Announce ourselves before the editor binding emits its first cursor // update. The server's beforeHandleAwareness hook overwrites `user` with // the authenticated identity. prov.setAwarenessField('user', {}) - prov.setAwarenessField(SEED_CAPABLE_KEY, !unref(effectiveReadOnly)) return { prov, clearConnectTimer } } @@ -1007,7 +1083,7 @@ export function useYjsSession(options: YjsSessionOptions): YjsSession { // non-null instance; nobody else will ever join, which is the point. aw = new Awareness(doc) status.value = YjsStatus.Local - void onProviderSynced(doc, null, aw) + void onProviderSynced(doc, null) } const meta = sessionMeta(doc) @@ -1021,6 +1097,7 @@ export function useYjsSession(options: YjsSessionOptions): YjsSession { onCleanup(() => { reporter.cancel() clearConnectTimer() + abandonSeedGrant() meta.map.unobserve(metaObserver) doc.off('update', reporter.onDocUpdate) if (prov) { diff --git a/packages/web-pkg/tests/unit/composables/yjs/useYjsSession.spec.ts b/packages/web-pkg/tests/unit/composables/yjs/useYjsSession.spec.ts index 6f484f5c5fa..e576ae14b19 100644 --- a/packages/web-pkg/tests/unit/composables/yjs/useYjsSession.spec.ts +++ b/packages/web-pkg/tests/unit/composables/yjs/useYjsSession.spec.ts @@ -32,36 +32,87 @@ interface MockProvider { name: string document: Y.Doc awareness: Awareness + stopped: boolean destroy: ReturnType disconnect: ReturnType detach: ReturnType setAwarenessField: ReturnType + sendStateless: ReturnType triggerSynced(): void triggerStatus(status: string): void triggerAuthFailed(reason: string): void + triggerStateless(payload: string): void } -const { providerInstances } = vi.hoisted(() => { - return { providerInstances: [] as MockProvider[] } +// `relay` turns the mock into a tiny in-memory server: doc updates and +// awareness fields travel between same-room providers with the given delays. +const { providerInstances, relay } = vi.hoisted(() => { + return { + providerInstances: [] as MockProvider[], + // `seedHolders` mirrors the Yjs server's seed registry (see + // `services/yjs/src/lib/seedGrant.ts`): one grant per room. + relay: { + enabled: false, + docDelayMs: 0, + awarenessDelayMs: 0, + seedHolders: new Map() + } + } }) vi.mock('@hocuspocus/provider', async () => { + const Yjs = await import('yjs') const { Awareness: AwarenessImpl } = await import('y-protocols/awareness') class MockHocuspocusProvider { url: string name: string document: Y.Doc awareness: Awareness + stopped = false // Mirrors the real provider, which destroys its own awareness. A bare spy // would hide a double-destroy in the session's cleanup. destroy = vi.fn(() => { + this.stopped = true this.awareness.destroy() }) - disconnect = vi.fn() + disconnect = vi.fn(() => { + this.stopped = true + }) // Present on the real provider and load-bearing: without it, doc updates // after a disconnect keep queueing in the websocket's `messageQueue`. detach = vi.fn() - setAwarenessField = vi.fn() + setAwarenessField = vi.fn((field: string, value: unknown) => { + if (!relay.enabled) return + this.awareness.setLocalStateField(field, value) + const clientId = this.document.clientID + setTimeout(() => { + const state = { ...this.awareness.getLocalState() } + for (const peer of this.peers()) peer.awareness.states.set(clientId, state) + }, relay.awarenessDelayMs) + }) + // The client asks the server who may seed. The answer comes from the + // server, not from a peer, so it does not depend on `relay.enabled`. It + // travels the same socket as a doc update, so it carries the same delay; a + // delay of 0 answers synchronously, which keeps every latency-agnostic + // test on a single `flushPromises`. + sendStateless = vi.fn((payload: string) => { + if (payload !== '_oc_seed_request') return + const socketId = String(this.document.clientID) + const answer = () => { + if (this.stopped) return + const holder = relay.seedHolders.get(this.name) + const granted = holder === undefined || holder === socketId + if (granted) relay.seedHolders.set(this.name, socketId) + this._opts.onStateless?.({ + payload: granted ? '_oc_seed_granted' : '_oc_seed_denied' + }) + } + if (relay.docDelayMs === 0) { + answer() + return + } + setTimeout(answer, relay.docDelayMs) + }) private _opts: any constructor(opts: any) { this.url = opts.url @@ -70,6 +121,22 @@ vi.mock('@hocuspocus/provider', async () => { this.awareness = new AwarenessImpl(opts.document) this._opts = opts providerInstances.push(this as MockProvider & MockHocuspocusProvider) + + // Remote updates carry the provider as origin, like the real one. + this.document.on('update', (update: Uint8Array, origin: unknown) => { + if (!relay.enabled || origin === this || this.stopped) return + setTimeout(() => { + for (const peer of this.peers()) { + if (peer.document.isDestroyed) continue + Yjs.applyUpdate(peer.document, update, peer) + } + }, relay.docDelayMs) + }) + } + private peers(): MockHocuspocusProvider[] { + return providerInstances.filter( + (p) => p !== (this as unknown as MockProvider) && p.name === this.name && !p.stopped + ) as unknown as MockHocuspocusProvider[] } triggerSynced() { this._opts.onSynced?.({ state: true }) @@ -80,6 +147,9 @@ vi.mock('@hocuspocus/provider', async () => { triggerAuthFailed(reason: string) { this._opts.onAuthenticationFailed?.({ reason }) } + triggerStateless(payload: string) { + this._opts.onStateless?.({ payload }) + } } return { HocuspocusProvider: MockHocuspocusProvider } }) @@ -209,6 +279,10 @@ function silenceConsoleError() { beforeEach(() => { providerInstances.length = 0 + relay.enabled = false + relay.docDelayMs = 0 + relay.awarenessDelayMs = 0 + relay.seedHolders.clear() }) afterEach(() => { @@ -336,14 +410,9 @@ describe('useYjsSession — local mode (no yjsServerUrl)', () => { expect(unref(s.session.status)).toBe('local') }) - it('hydrates the Y.Doc from currentContent (election degenerates to "we win")', async () => { - vi.useFakeTimers({ shouldAdvanceTime: true }) + it('hydrates the Y.Doc from currentContent without asking a server', async () => { const s = setupSession({ currentContent: 'hello local' }) await flushPromises() - // Local mode skips the remote-only 150ms awareness-settle wait and - // hydrates immediately; advancing timers here is just belt-and-braces. - vi.advanceTimersByTime(200) - await flushPromises() expect(s.ydoc).toBeTruthy() expect(s.ydoc!.getText(SHARED_TEXT_KEY).toString()).toBe('hello local') @@ -407,9 +476,10 @@ describe('useYjsSession — failed hydration', () => { expect(unref(s.session.isReady)).toBe(true) }) - // The seeding announce goes out before the hydrate so read-only peers drop - // their private copy in time. Left standing after a throw, it promises - // content that never arrives and every viewer stares at a blank room. + // The announce tells read-only peers to drop their private copy. Left + // standing after a throw, it promises content that never arrives and every + // viewer stares at a blank room. The lock also closes the socket, which + // makes the server pass the grant to another writer. it('withdraws the seeding announce so another peer can seed', async () => { vi.useFakeTimers({ shouldAdvanceTime: true }) const s = setupSession({ @@ -436,65 +506,115 @@ describe('useYjsSession — remote mode (yjsServerUrl set)', () => { expect(providerInstances[0].setAwarenessField).toHaveBeenCalledWith('user', {}) }) - // Regression: the election counted every awareness peer, but a read-only - // client never seeds - the server rejects its writes, it only hydrates a - // private copy. So whenever a viewer happened to hold the lower clientID, the - // editor deferred to it and nobody seeded: a blank editor over a file that is - // not blank, one keystroke away from being saved over the real content. - it('ignores read-only peers in the hydration election', async () => { - vi.useFakeTimers({ shouldAdvanceTime: true }) + // Regression: hydration used to be decided by an awareness election that + // counted every peer, but a read-only client never seeds - the server + // rejects its writes, it only hydrates a private copy. Whenever a viewer + // held the lower clientID the editor deferred to it and nobody seeded: a + // blank editor over a file that is not blank, one keystroke away from being + // saved over the real content. The server now decides, and it never grants + // to a read-only connection. + it('seeds an empty room while a read-only peer is connected', async () => { const s = setupSession({ yjsServerUrl: 'wss://example.test/yjs', currentContent: 'the real file body' }) await flushPromises() - // A read-only peer that won the election by holding the lower clientID. - const readOnlyPeer = s.ydoc!.clientID - 1 - providerInstances[0].awareness.states.set(readOnlyPeer, { - user: { id: 'viewer', name: 'Margaret Hamilton' }, - _oc_canSeed: false + providerInstances[0].awareness.states.set(s.ydoc!.clientID - 1, { + user: { id: 'viewer', name: 'Margaret Hamilton' } }) providerInstances[0].triggerSynced() - vi.advanceTimersByTime(200) await flushPromises() expect(s.ydoc!.getText(SHARED_TEXT_KEY).toString()).toBe('the real file body') }) - // The flip side: a peer that *can* seed still wins on the lower clientID, so - // two editors entering together do not both hydrate and duplicate the body. - it('still defers to a writable peer holding the lower clientID', async () => { - vi.useFakeTimers({ shouldAdvanceTime: true }) + it('asks the server for permission before seeding', async () => { const s = setupSession({ yjsServerUrl: 'wss://example.test/yjs', currentContent: 'the real file body' }) await flushPromises() - const writablePeer = s.ydoc!.clientID - 1 - providerInstances[0].awareness.states.set(writablePeer, { - user: { id: 'editor', name: 'Mary Kenneth Keller' }, - _oc_canSeed: true + providerInstances[0].triggerSynced() + await flushPromises() + + expect(providerInstances[0].sendStateless).toHaveBeenCalledWith('_oc_seed_request') + expect(s.ydoc!.getText(SHARED_TEXT_KEY).toString()).toBe('the real file body') + }) + + // The peer that holds the grant is seeding, and its body is on the way. + it('does not seed when the server denies the grant', async () => { + const s = setupSession({ + yjsServerUrl: 'wss://example.test/yjs', + currentContent: 'the real file body' }) + await flushPromises() + relay.seedHolders.set(providerInstances[0].name, 'a-peer') providerInstances[0].triggerSynced() - vi.advanceTimersByTime(200) await flushPromises() expect(s.ydoc!.getText(SHARED_TEXT_KEY).toString()).toBe('') + expect(unref(s.session.isReady)).toBe(true) }) - it('announces whether it can seed the room', async () => { - setupSession({ yjsServerUrl: 'wss://example.test/yjs', isReadOnly: true }) + // Regression: a reconnect inside the seed wait started a second hydration + // next to the one still waiting, and the first one never settled. + it('asks again when the socket reconnects inside the seed wait, and seeds once', async () => { + vi.useFakeTimers() + relay.docDelayMs = 100 + const s = setupSession({ + yjsServerUrl: 'wss://example.test/yjs', + currentContent: 'the file body' + }) await flushPromises() - expect(providerInstances[0].setAwarenessField).toHaveBeenCalledWith('_oc_canSeed', false) - providerInstances.length = 0 - setupSession({ yjsServerUrl: 'wss://example.test/yjs' }) + providerInstances[0].triggerSynced() + await vi.advanceTimersByTimeAsync(20) + providerInstances[0].triggerSynced() + await vi.advanceTimersByTimeAsync(1_000) + await flushPromises() + + expect(providerInstances[0].sendStateless).toHaveBeenCalledTimes(2) + expect(s.ydoc!.getText(SHARED_TEXT_KEY).toString()).toBe('the file body') + expect(unref(s.session.isReady)).toBe(true) + }) + + it('never asks for a grant on a read-only client', async () => { + const s = setupSession({ + yjsServerUrl: 'wss://example.test/yjs', + currentContent: 'the file body', + isReadOnly: true + }) await flushPromises() - expect(providerInstances[0].setAwarenessField).toHaveBeenCalledWith('_oc_canSeed', true) + + providerInstances[0].triggerSynced() + await flushPromises() + + expect(providerInstances[0].sendStateless).not.toHaveBeenCalled() + expect(s.ydoc!.getText(SHARED_TEXT_KEY).toString()).toBe('the file body') + }) + + // Seeding is a first-entry decision. A reconnect re-fires `onSynced`; an + // empty doc at that point is one the user emptied, not an unseeded room. + it('does not seed again on a mid-session reconnect into an emptied doc', async () => { + const s = setupSession({ + yjsServerUrl: 'wss://example.test/yjs', + currentContent: 'the file body' + }) + await flushPromises() + providerInstances[0].triggerSynced() + await flushPromises() + + const yText = s.ydoc!.getText(SHARED_TEXT_KEY) + yText.delete(0, yText.length) + providerInstances[0].triggerSynced() + await flushPromises() + + expect(providerInstances[0].sendStateless).toHaveBeenCalledTimes(1) + expect(yText.toString()).toBe('') }) it('does not hydrate until onSynced fires (remote waits for the server)', async () => { @@ -627,8 +747,6 @@ describe('useYjsSession — remote mode (yjsServerUrl set)', () => { }) await flushPromises() providerInstances[0].triggerSynced() - // Past the 150ms hydration-election wait. - vi.advanceTimersByTime(200) await flushPromises() expect(unref(s.session.isReady)).toBe(true) @@ -1881,3 +1999,168 @@ describe('useYjsSession — cleanup', () => { expect(providerInstances).toHaveLength(0) }) }) + +// Regression: two editors opening the same file at once (#3141). An awareness +// election let both seed whenever the round-trip outlived its window, and the +// body then showed twice for everyone. The Yjs server now grants seeding to +// exactly one connection per room, so no wait has to be guessed. +describe('useYjsSession — simultaneous hydration', () => { + const FILE_BODY = 'the file body' + const URL = 'wss://example.test/yjs' + + async function bothOpenSimultaneously() { + vi.useFakeTimers() + relay.enabled = true + const a = setupSession({ yjsServerUrl: URL, currentContent: FILE_BODY }) + const b = setupSession({ yjsServerUrl: URL, currentContent: FILE_BODY }) + await flushPromises() + + providerInstances[0].triggerSynced() + providerInstances[1].triggerSynced() + + // Well past both relay hops and the seed answer. + await vi.advanceTimersByTimeAsync(2_000) + await flushPromises() + + return { a, b } + } + + function text(s: ReturnType) { + return s.ydoc!.getText(SHARED_TEXT_KEY).toString() + } + + // The mock server's registry knows who was granted, the doc only that someone was. + function seeder(a: ReturnType, b: ReturnType) { + return relay.seedHolders.get(providerInstances[0].name) === String(a.ydoc!.clientID) ? a : b + } + + // A doubled body is not only wrong on screen: reported to the caller it is + // diffed against the file, marks it dirty and is one manual save away from + // being written to disk. 600ms outlives SERIALIZE_DEBOUNCE_MS, so a report + // would have time to get out. + it.each([20, 600])('seeds once with a %ims seed answer', async (docDelayMs) => { + relay.awarenessDelayMs = 300 + relay.docDelayMs = docDelayMs + + const { a, b } = await bothOpenSimultaneously() + + const doubled = `${FILE_BODY}${FILE_BODY}` + expect(text(a)).toBe(FILE_BODY) + expect(text(b)).toBe(FILE_BODY) + expect(a.onContentChange).not.toHaveBeenCalledWith(doubled) + expect(b.onContentChange).not.toHaveBeenCalledWith(doubled) + expect(a.ydoc!.getMap('_oc_meta').get('hydrated')).toBe(true) + expect(b.ydoc!.getMap('_oc_meta').get('hydrated')).toBe(true) + }) + + it('keeps the client that did not seed editable', async () => { + relay.awarenessDelayMs = 300 + relay.docDelayMs = 20 + + const { a, b } = await bothOpenSimultaneously() + const other = seeder(a, b) === a ? b : a + + other.ydoc!.getText(SHARED_TEXT_KEY).insert(0, 'edit ') + await vi.advanceTimersByTimeAsync(1_000) + + expect(text(a)).toBe(`edit ${FILE_BODY}`) + expect(text(b)).toBe(`edit ${FILE_BODY}`) + expect(other.onContentChange).toHaveBeenCalledWith(`edit ${FILE_BODY}`) + }) +}) + +// The grant holder can leave before it seeds. The server then hands the grant +// to someone still in the room, unasked. +describe('useYjsSession — unsolicited seed grant', () => { + const URL = 'wss://example.test/yjs' + + it('seeds an empty room on a grant that arrives on its own', async () => { + const s = setupSession({ yjsServerUrl: URL, currentContent: 'the file body' }) + await flushPromises() + relay.seedHolders.set(providerInstances[0].name, 'a-peer') + + providerInstances[0].triggerSynced() + await flushPromises() + expect(s.ydoc!.getText(SHARED_TEXT_KEY).toString()).toBe('') + + providerInstances[0].triggerStateless('_oc_seed_granted') + await flushPromises() + + expect(s.ydoc!.getText(SHARED_TEXT_KEY).toString()).toBe('the file body') + expect(s.ydoc!.getMap('_oc_meta').get('hydrated')).toBe(true) + }) + + // A grant for a room that already has content is normal: the server does not + // track whether the previous holder seeded, the client checks instead. + it('ignores a grant once the room has content', async () => { + const s = setupSession({ yjsServerUrl: URL, currentContent: 'the file body' }) + await flushPromises() + providerInstances[0].triggerSynced() + await flushPromises() + + providerInstances[0].triggerStateless('_oc_seed_granted') + await flushPromises() + + expect(s.ydoc!.getText(SHARED_TEXT_KEY).toString()).toBe('the file body') + expect(s.onContentChange).not.toHaveBeenCalled() + }) + + // The session is live by then, so without a guard the seed would be reported + // as an edit and mark an untouched file dirty. + it('does not report the late seed as content', async () => { + vi.useFakeTimers() + const s = setupSession({ yjsServerUrl: URL, currentContent: 'the file body' }) + await flushPromises() + relay.seedHolders.set(providerInstances[0].name, 'a-peer') + providerInstances[0].triggerSynced() + await flushPromises() + + providerInstances[0].triggerStateless('_oc_seed_granted') + await vi.advanceTimersByTimeAsync(1_000) + + expect(s.ydoc!.getText(SHARED_TEXT_KEY).toString()).toBe('the file body') + expect(s.onContentChange).not.toHaveBeenCalled() + }) + + // Same outcome as a failed initial hydration: the lock closes the socket, + // so the server passes the grant on instead of leaving it with a client + // that cannot use it. + it('locks the session when the late seed throws', async () => { + silenceConsoleError() + const throwingAdapter: YjsAdapter = { + ...testAdapter, + hydrate() { + throw new Error('adapter blew up') + } + } + const s = setupSession({ + yjsServerUrl: URL, + currentContent: 'the file body', + adapter: throwingAdapter + }) + await flushPromises() + relay.seedHolders.set(providerInstances[0].name, 'a-peer') + providerInstances[0].triggerSynced() + await flushPromises() + + providerInstances[0].triggerStateless('_oc_seed_granted') + await flushPromises() + + expect(unref(s.session.isLockedForReload)).toBe(true) + expect(providerInstances[0].disconnect).toHaveBeenCalled() + expect(s.ydoc!.getMap('_oc_meta').get('hydrated')).toBeUndefined() + }) + + it('ignores a grant on a read-only client', async () => { + const s = setupSession({ yjsServerUrl: URL, currentContent: 'the file body', isReadOnly: true }) + await flushPromises() + providerInstances[0].triggerSynced() + await flushPromises() + s.ydoc!.getText(SHARED_TEXT_KEY).delete(0, s.ydoc!.getText(SHARED_TEXT_KEY).length) + + providerInstances[0].triggerStateless('_oc_seed_granted') + await flushPromises() + + expect(s.ydoc!.getMap('_oc_meta').get('hydrated')).toBeUndefined() + }) +}) diff --git a/services/yjs/README.md b/services/yjs/README.md index 2359cd9fc1b..2fad692c2ae 100644 --- a/services/yjs/README.md +++ b/services/yjs/README.md @@ -15,6 +15,18 @@ Every connection is authenticated and authorized against OpenCloud: - write access is derived from the effective permission actions on the file - awareness states are re-stamped with the authenticated identity, so users cannot spoof each other +## Seeding an empty room + +The first client in a room writes the file body into the Y.Doc. Exactly one client may do that, or +the room ends up holding the body twice. The yjs server decides which, because it is the only party +that sees every connection to a room at once. A client asks over a stateless message, and gets a +grant or a refusal. Read-only connections are always refused. + +Grants live in process memory and last as long as the holder's connection. If the holder leaves +before it seeds, the grant passes to another writer in the room. A grant for a room that already has +content is harmless: it is permission, not an instruction, and the client checks its own document +first. + ## Configuration | Variable | Default | Description | diff --git a/services/yjs/src/lib/hooks.ts b/services/yjs/src/lib/hooks.ts index 664411a19fc..bfcc2f29089 100644 --- a/services/yjs/src/lib/hooks.ts +++ b/services/yjs/src/lib/hooks.ts @@ -1,4 +1,10 @@ -import type { Extension } from '@hocuspocus/server' +import type { + afterUnloadDocumentPayload, + Document, + Extension, + onDisconnectPayload, + onStatelessPayload +} from '@hocuspocus/server' import { deterministicColor } from './color.ts' import { DeniedReason, isRefusal, refuse } from './errors.ts' import { @@ -6,16 +12,10 @@ import { probeFileAccess, validateTokenAgainstOpenCloud } from './graph.ts' +import { createSeedRegistry, SeedMessage } from './seedGrant.ts' export const HEALTH_ENDPOINT_PATH = '/healthz/ready' -/** - * Awareness field marking a connection as able to seed an empty room. Read by - * the client's hydration election in `useYjsSession`; the name has - * to match `SEED_CAPABLE_KEY` there. - */ -export const SEED_CAPABLE_KEY = '_oc_canSeed' - export type YjsUser = { id: string displayName: string @@ -105,6 +105,21 @@ export async function authenticate( } export function createHooks({ opencloudUrl, lifecycle }: HookOptions) { + const seedRegistry = createSeedRegistry() + + /** + * Hand the seed grant to another writer in the room. Called when the holder + * leaves, which may have happened before it seeded. + */ + function passSeedGrantOn(document: Document, documentName: string): void { + const writer = document.getConnections().find((connection) => !connection.readOnly) + if (!writer) { + return + } + seedRegistry.grantTo(documentName, writer.socketId) + writer.sendStateless(SeedMessage.Granted) + } + return { async onRequest({ request, response }: RequestPayload): Promise { const requestPath = request.url?.split('?')[0] ?? '/' @@ -157,14 +172,37 @@ export function createHooks({ opencloudUrl, lifecycle }: HookOptions) { async onDisconnect({ documentName, - clientsCount - }: { - documentName: string - clientsCount: number - }): Promise { + clientsCount, + socketId, + document + }: onDisconnectPayload): Promise { console.log( `[onDisconnect] document=${JSON.stringify(documentName)} remaining=${clientsCount}` ) + if (seedRegistry.release(documentName, socketId)) { + passSeedGrantOn(document, documentName) + } + }, + + /** + * Safety net only: the holder's `onDisconnect` has normally released the + * grant by the time the room unloads. + */ + async afterUnloadDocument({ documentName }: afterUnloadDocumentPayload): Promise { + seedRegistry.forget(documentName) + }, + + /** + * Seeding arbitration, see `seedGrant.ts`. The answer is permission, not + * an instruction: the client still checks its own document first, so a + * grant for a room that already has content costs nothing. + */ + async onStateless({ connection, documentName, payload }: onStatelessPayload): Promise { + if (payload !== SeedMessage.Request) { + return + } + const granted = seedRegistry.request(documentName, connection.socketId, connection.readOnly) + connection.sendStateless(granted ? SeedMessage.Granted : SeedMessage.Denied) }, /** @@ -182,11 +220,8 @@ export function createHooks({ opencloudUrl, lifecycle }: HookOptions) { name: user.displayName, color: user.color } - const readOnly = Boolean(context?.readOnly ?? connection?.context?.readOnly) for (const state of states.values()) { state.user = canonical - // Whether this connection may seed (hydrate) an empty room - state[SEED_CAPABLE_KEY] = !readOnly } } } satisfies Extension diff --git a/services/yjs/src/lib/seedGrant.ts b/services/yjs/src/lib/seedGrant.ts new file mode 100644 index 00000000000..144094b0966 --- /dev/null +++ b/services/yjs/src/lib/seedGrant.ts @@ -0,0 +1,67 @@ +/** + * Stateless protocol for seeding an empty room: exactly one connection per + * room is allowed to write the initial body. + * + * The payload strings have to match the `SEED_*` constants in the client's + * `useYjsSession.ts`, keep the two in sync. + */ +export const SeedMessage = { + Request: '_oc_seed_request', + Granted: '_oc_seed_granted', + Denied: '_oc_seed_denied' +} as const + +/** + * Which connection currently holds the right to seed each room. In memory on + * purpose: the question is only ever about connections that are live right + * now, so the answer has to die with the room. + * + * A grant lasts as long as the holder's socket. A writer that is granted but + * never seeds blocks the room until it disconnects. + */ +export function createSeedRegistry() { + const holders = new Map() + + /** Answer a seed request. Read-only connections are refused, no write permissions. */ + function request(documentName: string, socketId: string, readOnly: boolean): boolean { + if (readOnly) { + return false + } + const holder = holders.get(documentName) + if (holder !== undefined && holder !== socketId) { + return false + } + holders.set(documentName, socketId) + return true + } + + /** Hand the grant to a connection that did not ask, see `release`. */ + function grantTo(documentName: string, socketId: string): void { + holders.set(documentName, socketId) + } + + /** + * Give up a leaving connection's grant. True when the leaver held it, which + * means the grant has to move on: a client that took it and left without + * seeding would otherwise leave the room empty for everyone. + * + * Passing the grant to a room that already has content is harmless, because + * the client checks its own document before using a grant. + */ + function release(documentName: string, socketId: string): boolean { + if (holders.get(documentName) !== socketId) { + return false + } + holders.delete(documentName) + return true + } + + /** Forget a room that no longer exists. */ + function forget(documentName: string): void { + holders.delete(documentName) + } + + return { request, grantTo, release, forget } +} + +export type SeedRegistry = ReturnType diff --git a/services/yjs/tests/unit/lib/hooks.spec.ts b/services/yjs/tests/unit/lib/hooks.spec.ts index 50e846eb0a5..2fc81289e07 100644 --- a/services/yjs/tests/unit/lib/hooks.spec.ts +++ b/services/yjs/tests/unit/lib/hooks.spec.ts @@ -1,5 +1,6 @@ import type { MockInstance } from 'vitest' -import { createHooks, HEALTH_ENDPOINT_PATH, SEED_CAPABLE_KEY } from '../../../src/lib/hooks.ts' +import { createHooks, HEALTH_ENDPOINT_PATH } from '../../../src/lib/hooks.ts' +import { SeedMessage } from '../../../src/lib/seedGrant.ts' import { DeniedReason, refuse } from '../../../src/lib/errors.ts' import * as graph from '../../../src/lib/graph.ts' @@ -25,6 +26,38 @@ function getResponse() { return { writeHead: vi.fn(), end: vi.fn() } } +type FakeConnection = { + socketId: string + readOnly: boolean + sendStateless: ReturnType +} + +function connection(socketId: string, readOnly = false): FakeConnection { + return { socketId, readOnly, sendStateless: vi.fn() } +} + +function statelessPayload( + conn: FakeConnection, + documentName = 'doc', + payload: string = SeedMessage.Request +) { + return { connection: conn, documentName, payload } as any +} + +function disconnectPayload({ + documentName = 'doc', + clientsCount = 0, + socketId = 'a', + remaining = [] as FakeConnection[] +} = {}) { + return { + documentName, + clientsCount, + socketId, + document: { getConnections: () => remaining } + } as any +} + let logSpy: MockInstance beforeEach(() => { @@ -289,16 +322,8 @@ describe('beforeHandleAwareness', () => { await getHooks().beforeHandleAwareness({ states, context: { readOnly: false, user } }) - expect(states.get(1)).toEqual({ user: canonical, cursor: 1, [SEED_CAPABLE_KEY]: true }) - expect(states.get(2)).toEqual({ user: canonical, [SEED_CAPABLE_KEY]: true }) - }) - - it('marks a read-only connection as not seed capable', async () => { - const states = new Map>([[1, { [SEED_CAPABLE_KEY]: true }]]) - - await getHooks().beforeHandleAwareness({ states, context: { readOnly: true, user } }) - - expect(states.get(1)![SEED_CAPABLE_KEY]).toBe(false) + expect(states.get(1)).toEqual({ user: canonical, cursor: 1 }) + expect(states.get(2)).toEqual({ user: canonical }) }) it('falls back to the connection context', async () => { @@ -310,7 +335,7 @@ describe('beforeHandleAwareness', () => { connection: { context: { readOnly: true, user } } }) - expect(states.get(1)).toEqual({ user: canonical, [SEED_CAPABLE_KEY]: false }) + expect(states.get(1)).toEqual({ user: canonical }) }) it('leaves the states untouched when no user is known', async () => { @@ -349,8 +374,130 @@ describe('logging hooks', () => { }) it('logs the remaining client count on disconnect', async () => { - await getHooks().onDisconnect({ documentName: 'doc', clientsCount: 2 }) + await getHooks().onDisconnect( + disconnectPayload({ documentName: 'doc', clientsCount: 2, socketId: 'other' }) + ) expect(logSpy).toHaveBeenCalledWith('[onDisconnect] document="doc" remaining=2') }) }) + +// The server arbitrates who seeds an empty room, because peers cannot agree +// on it among themselves - Yjs never echoes an update back to its sender. +describe('seeding arbitration', () => { + it('grants the first writer and denies the next', async () => { + const hooks = getHooks() + const first = connection('a') + const second = connection('b') + + await hooks.onStateless(statelessPayload(first)) + await hooks.onStateless(statelessPayload(second)) + + expect(first.sendStateless).toHaveBeenCalledWith(SeedMessage.Granted) + expect(second.sendStateless).toHaveBeenCalledWith(SeedMessage.Denied) + }) + + // The server rejects their writes, so a read-only grantee would leave the + // room empty for everyone. + it('denies a read-only connection', async () => { + const conn = connection('a', true) + + await getHooks().onStateless(statelessPayload(conn)) + + expect(conn.sendStateless).toHaveBeenCalledWith(SeedMessage.Denied) + }) + + it('answers the holder again with a grant', async () => { + const hooks = getHooks() + const conn = connection('a') + + await hooks.onStateless(statelessPayload(conn)) + await hooks.onStateless(statelessPayload(conn)) + + expect(conn.sendStateless).toHaveBeenNthCalledWith(2, SeedMessage.Granted) + }) + + it('grants each room separately', async () => { + const hooks = getHooks() + const first = connection('a') + const second = connection('b') + + await hooks.onStateless(statelessPayload(first, 'doc-1')) + await hooks.onStateless(statelessPayload(second, 'doc-2')) + + expect(first.sendStateless).toHaveBeenCalledWith(SeedMessage.Granted) + expect(second.sendStateless).toHaveBeenCalledWith(SeedMessage.Granted) + }) + + it('ignores an unrelated stateless payload', async () => { + const conn = connection('a') + + await getHooks().onStateless(statelessPayload(conn, 'doc', 'something-else')) + + expect(conn.sendStateless).not.toHaveBeenCalled() + }) + + // The holder may have left before seeding, and nobody else is allowed to. + it('passes the grant on when the holder leaves', async () => { + const hooks = getHooks() + const holder = connection('a') + const peer = connection('b') + + await hooks.onStateless(statelessPayload(holder)) + await hooks.onDisconnect(disconnectPayload({ socketId: 'a', remaining: [peer] })) + + expect(peer.sendStateless).toHaveBeenCalledWith(SeedMessage.Granted) + }) + + it('skips read-only peers when passing the grant on', async () => { + const hooks = getHooks() + const holder = connection('a') + const viewer = connection('b', true) + const writer = connection('c') + + await hooks.onStateless(statelessPayload(holder)) + await hooks.onDisconnect(disconnectPayload({ socketId: 'a', remaining: [viewer, writer] })) + + expect(viewer.sendStateless).not.toHaveBeenCalled() + expect(writer.sendStateless).toHaveBeenCalledWith(SeedMessage.Granted) + }) + + it('passes the grant on to exactly one writer', async () => { + const hooks = getHooks() + const holder = connection('a') + const first = connection('b') + const second = connection('c') + + await hooks.onStateless(statelessPayload(holder)) + await hooks.onDisconnect(disconnectPayload({ socketId: 'a', remaining: [first, second] })) + + expect(first.sendStateless).toHaveBeenCalledWith(SeedMessage.Granted) + expect(second.sendStateless).not.toHaveBeenCalled() + // The new holder is on record: a later request from the other writer is refused. + await hooks.onStateless(statelessPayload(second)) + expect(second.sendStateless).toHaveBeenCalledWith(SeedMessage.Denied) + }) + + it('keeps the grant when someone other than the holder leaves', async () => { + const hooks = getHooks() + const holder = connection('a') + const peer = connection('b') + + await hooks.onStateless(statelessPayload(holder)) + await hooks.onDisconnect(disconnectPayload({ socketId: 'b', remaining: [peer] })) + + expect(peer.sendStateless).not.toHaveBeenCalled() + }) + + it('grants again once the room is gone', async () => { + const hooks = getHooks() + const first = connection('a') + const second = connection('b') + + await hooks.onStateless(statelessPayload(first)) + await hooks.afterUnloadDocument({ documentName: 'doc' } as any) + await hooks.onStateless(statelessPayload(second)) + + expect(second.sendStateless).toHaveBeenCalledWith(SeedMessage.Granted) + }) +}) diff --git a/tests/e2e/features/yjs/collaborativeEditing.feature b/tests/e2e/features/yjs/collaborativeEditing.feature index 0d64ee2bb24..d6a2bc91e31 100644 --- a/tests/e2e/features/yjs/collaborativeEditing.feature +++ b/tests/e2e/features/yjs/collaborativeEditing.feature @@ -31,7 +31,7 @@ Feature: yjs collaborative editing And "Carol" opens file "example.md" via "text-editor" using the context menu And "Carol" should not see a yjs status And "Carol" is in a text-editor - And "Carol" should see the text "Brian says hello" in the text-editor + And "Carol" should see the text "Brian says hello" exactly once in the text-editor And "Carol" should not be able to edit the current file And "Alice" logs in @@ -41,7 +41,7 @@ Feature: yjs collaborative editing | status | | Connected | And "Alice" is in a text-editor - Then "Alice" should see the text "Brian says hello" in the text-editor + Then "Alice" should see the text "Brian says hello" exactly once in the text-editor When "Alice" enters the text "Alice says hello" in editor "TextEditor" Then "Brian" should see the text "Alice says hello" in the text-editor @@ -181,8 +181,8 @@ Feature: yjs collaborative editing And "Alice" opens file "example.md" via "text-editor" using the context menu And "Alice" is in a text-editor - Then "Alice" should see the text "lorem ipsum" in the text-editor - And "Brian" should see the text "lorem ipsum" in the text-editor + Then "Alice" should see the text "lorem ipsum" exactly once in the text-editor + And "Brian" should see the text "lorem ipsum" exactly once in the text-editor When "Alice" enters the text "Alice says hello" in editor "TextEditor" Then "Brian" should see the text "Alice says hello" in the text-editor diff --git a/tests/e2e/features/yjs/hydration.feature b/tests/e2e/features/yjs/hydration.feature new file mode 100644 index 00000000000..15e1a60c5d2 --- /dev/null +++ b/tests/e2e/features/yjs/hydration.feature @@ -0,0 +1,68 @@ +Feature: yjs hydration + As a user + I want the file content to appear only once when a session starts + So that joining a session never duplicates the document body + + Scenario: two writers hydrate the same file at the same time + Given "Admin" creates following users using API + | id | + | Alice | + | Brian | + And "Alice" creates the following file into personal space using API + | pathToFile | content | + | example.md | lorem ipsum | + And "Alice" shares the following resources using API + | resource | recipient | type | role | + | example.md | Brian | user | Can edit | + + And "Alice" logs in + And "Alice" opens the "files" app + And "Brian" logs in + And "Brian" navigates to the shared with me page + + When the following users open file "example.md" via "text-editor" using the context menu at the same time + | id | + | Alice | + | Brian | + Then "Alice" should see the following yjs status + | status | + | Connected | + And "Brian" should see the following yjs status + | status | + | Connected | + And "Alice" should see the text "lorem ipsum" exactly once in the text-editor + And "Brian" should see the text "lorem ipsum" exactly once in the text-editor + + And "Alice" logs out + And "Brian" logs out + + Scenario: a writer and a reader hydrate the same file at the same time + Given "Admin" creates following users using API + | id | + | Alice | + | Brian | + And "Alice" creates the following file into personal space using API + | pathToFile | content | + | example.md | lorem ipsum | + And "Alice" shares the following resources using API + | resource | recipient | type | role | + | example.md | Brian | user | Can view | + + And "Alice" logs in + And "Alice" opens the "files" app + And "Brian" logs in + And "Brian" navigates to the shared with me page + + When the following users open file "example.md" via "text-editor" using the context menu at the same time + | id | + | Alice | + | Brian | + Then "Alice" should see the following yjs status + | status | + | Connected | + And "Brian" should not see a yjs status + And "Alice" should see the text "lorem ipsum" exactly once in the text-editor + And "Brian" should see the text "lorem ipsum" exactly once in the text-editor + + And "Alice" logs out + And "Brian" logs out diff --git a/tests/e2e/steps/ui/resources.ts b/tests/e2e/steps/ui/resources.ts index 85511bdf4ef..09c7de86e81 100644 --- a/tests/e2e/steps/ui/resources.ts +++ b/tests/e2e/steps/ui/resources.ts @@ -19,6 +19,9 @@ import { File } from '../../support/types' import { waitProcessingToFinish } from '../../support/objects/app-files/fileEvents' import { editor } from '../../support/objects/app-files/utils' +// how long a late duplicate of the hydrated content may need to arrive +const duplicateContentGraceMs = 3000 + When( '{string} creates the following resource(s)', async ({ world }: { world: World }, stepUser: string, stepTable: DataTable): Promise => { @@ -1271,6 +1274,16 @@ Then( } ) +const allowedFileViewers = ['collabora-online', 'text-editor', 'preview'] as const +type AllowedFileViewer = (typeof allowedFileViewers)[number] + +function toFileViewer(fileViewer: string): AllowedFileViewer { + if (!allowedFileViewers.includes(fileViewer as AllowedFileViewer)) { + throw new Error(`Unsupported file viewer: ${fileViewer}`) + } + return fileViewer as AllowedFileViewer +} + When( '{string} opens file {string} via {string} using the context menu', async ( @@ -1279,15 +1292,30 @@ When( file: string, fileViewer: string ): Promise => { - const allowedViewers = ['collabora-online', 'text-editor', 'preview'] as const - - if (!allowedViewers.includes(fileViewer as any)) { - throw new Error(`Unsupported file viewer: ${fileViewer}`) - } const { page } = world.actorsEnvironment.getActor({ key: stepUser }) const resourceObject = new objects.applicationFiles.Resource({ page }) - await resourceObject.openFileViaContextMenu(file, fileViewer as (typeof allowedViewers)[number]) + await resourceObject.openFileViaContextMenu(file, toFileViewer(fileViewer)) + } +) + +When( + 'the following users open file {string} via {string} using the context menu at the same time', + async ( + { world }: { world: World }, + file: string, + fileViewer: string, + stepTable: DataTable + ): Promise => { + const viewer = toFileViewer(fileViewer) + + await Promise.all( + stepTable.hashes().map(({ id }) => { + const { page } = world.actorsEnvironment.getActor({ key: id }) + const resourceObject = new objects.applicationFiles.Resource({ page }) + return resourceObject.openFileViaContextMenu(file, viewer) + }) + ) } ) @@ -1467,6 +1495,23 @@ Then( } ) +Then( + '{string} should see the text {string} exactly once in the text-editor', + async ({ world }: { world: World }, stepUser: string, text: string): Promise => { + const { page } = world.actorsEnvironment.getActor({ key: stepUser }) + const content = editor.textEditorContentLocator(page) + await expect(content).toContainText(text) + + // a duplicate seed lands shortly after hydration, so the count has to stay at one + const deadline = Date.now() + duplicateContentGraceMs + for (;;) { + expect(await editor.countTextOccurrences(content, text)).toBe(1) + if (Date.now() >= deadline) break + await page.waitForTimeout(250) + } + } +) + Then( '{string} should see an error message', async ({ world }: { world: World }, stepUser: string, errorMessage: string): Promise => { diff --git a/tests/e2e/support/objects/app-files/utils/editor.ts b/tests/e2e/support/objects/app-files/utils/editor.ts index 997ec0b50c2..079ca79742c 100644 --- a/tests/e2e/support/objects/app-files/utils/editor.ts +++ b/tests/e2e/support/objects/app-files/utils/editor.ts @@ -74,6 +74,9 @@ export const yjsStatusLocator = (page: Page, status: string): Locator => export const collaborationCaretLocator = (page: Page, displayName: string): Locator => page.locator(collaborationCursorLabel, { hasText: displayName }) +export const countTextOccurrences = async (locator: Locator, text: string): Promise => + (await locator.innerText()).split(text).length - 1 + export const resolveSaveConflict = async (page: Page, action: string): Promise => { const button = saveConflictDialogButtons[action] if (!button) { From d6a4e8e7d6993c7b5bfb0ae2ec388f146f53fb77 Mon Sep 17 00:00:00 2001 From: Jannik Stehle Date: Tue, 8 Sep 2026 12:58:32 +0200 Subject: [PATCH 2/2] docs: update known limits in yjs.md --- dev/docs/yjs.md | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/dev/docs/yjs.md b/dev/docs/yjs.md index 3418df6aaee..6ea352ad474 100644 --- a/dev/docs/yjs.md +++ b/dev/docs/yjs.md @@ -347,7 +347,7 @@ A `Y.Map` alongside the editor content, used for coordination the editor never s | `isStale` | any writer | the file changed outside this room; rehydrate | | `nativeEtag` | writer that noticed | the etag recovery should settle on | | `recoveryClientId` | writer that noticed | which peer is elected to re-seed the room | -| `hydrated` | the seeding peer | someone is seeding the room right now | +| `hydrated` | the seeding peer | this room has been seeded | Every key lives in the shared Y.Doc, so a read-only peer's writes to it are rejected by the Yjs server along with everything else. Staleness noticed by a @@ -454,10 +454,6 @@ saved N times per autosave interval. There is election for hydration but none fo `beforeunload` and the unsaved-changes modal fire for edits you did not make. Writers only - `isDirty` is hard-wired to `false` for a read-only client, which has nothing to save. -**Hydration election can race.** The 150 ms awareness-settle window is heuristic. If awareness has not propagated in -time, two peers can both elect themselves and hydrate, duplicating content. Server-side hydration would remove the whole -class. - **`_oc_meta` is writable by any client with write access.** A buggy or hostile client can set `isStale` and reset every peer in the room. The room's control plane has no server authority beyond the read-only gate. @@ -489,10 +485,6 @@ peer's stamp that is still in flight. An external writer - a desktop sync client conflict dialog instead of being overwritten. The residual gap is timing: a peer save whose stamp arrives after the grace period turns into a spurious conflict dialog. -**`_oc_meta.hydrated` is never cleared.** Stale recovery deletes `isStale`, `nativeEtag` and `recoveryClientId` but -leaves `hydrated` set. Harmless today, since the recovered room really is seeded, but it means the flag tracks "this -room was ever seeded" rather than "a peer is seeding right now". - **Stale recovery needs someone who holds the fresh body.** Only a peer whose fetched etag matches `nativeEtag` may re-seed the room. If that peer leaves before finishing, the room stays flagged until another client opens the file and picks the job up. Peers already in the room keep editing a document that no longer matches disk in the meantime.