From 3d646d3fd5f0c45210b49078854d67e2217d1e36 Mon Sep 17 00:00:00 2001 From: Jason Stewart <4491509+jasonestewart@users.noreply.github.com> Date: Mon, 6 Jul 2026 14:08:57 +0100 Subject: [PATCH] feat(collaborator): add appendContent RPC for merge (non-clobbering) writes The collaborator service only exposes full-replace writes (createContent / updateContent). updateContent clears the document's Y.XmlFragment before applying the incoming update, so a read-modify-write from an automation silently clobbers any concurrent edits (e.g. a human editing the same Document in the browser). Add an appendContent RPC that is identical to updateContent but omits the `fragment.delete(0, fragment.length)` step. Applying the incoming update onto the untouched fragment lets the Y.js CRDT merge the new content in, giving append semantics that are race-free against concurrent editors (the mutation runs inside the same hocuspocus direct connection). - server/collaborator: new appendContent method + dispatch registration - collaborator-client: AppendContentRequest/Response types + appendMarkup() - tests: append-to-empty, append-preserves-prior, replace-vs-append, and two-independent-appends-both-land (CRDT merge, no clobber) Co-Authored-By: Claude Opus 4.8 (1M context) --- .../collaborator-client/src/client.ts | 24 +++++ .../src/__tests__/appendContent.test.ts | 91 +++++++++++++++++++ .../src/rpc/methods/appendContent.ts | 67 ++++++++++++++ server/collaborator/src/rpc/methods/index.ts | 4 +- 4 files changed, 185 insertions(+), 1 deletion(-) create mode 100644 server/collaborator/src/__tests__/appendContent.test.ts create mode 100644 server/collaborator/src/rpc/methods/appendContent.ts diff --git a/foundations/core/packages/collaborator-client/src/client.ts b/foundations/core/packages/collaborator-client/src/client.ts index 8cabaf1285c..b93314f3b95 100644 --- a/foundations/core/packages/collaborator-client/src/client.ts +++ b/foundations/core/packages/collaborator-client/src/client.ts @@ -45,11 +45,21 @@ export interface UpdateContentRequest { // eslint-disable-next-line @typescript-eslint/no-empty-interface export interface UpdateContentResponse {} +/** @public */ +export interface AppendContentRequest { + content: Record +} + +/** @public */ +// eslint-disable-next-line @typescript-eslint/no-empty-interface +export interface AppendContentResponse {} + /** @public */ export interface CollaboratorClient { getMarkup: (document: CollaborativeDoc, source?: Ref | null) => Promise createMarkup: (document: CollaborativeDoc, markup: Markup) => Promise updateMarkup: (document: CollaborativeDoc, markup: Markup) => Promise + appendMarkup: (document: CollaborativeDoc, markup: Markup) => Promise copyContent: (source: CollaborativeDoc, target: CollaborativeDoc) => Promise } @@ -140,6 +150,20 @@ class CollaboratorClientImpl implements CollaboratorClient { ) } + async appendMarkup (document: CollaborativeDoc, markup: Markup): Promise { + const content = { + [document.objectAttr]: markup + } + + await retry( + 3, + async () => { + await this.rpc(document, 'appendContent', { content }) + }, + 50 + ) + } + async copyContent (source: CollaborativeDoc, target: CollaborativeDoc, content?: Ref): Promise { const markup = await this.getMarkup(source, content) await this.updateMarkup(target, markup) diff --git a/server/collaborator/src/__tests__/appendContent.test.ts b/server/collaborator/src/__tests__/appendContent.test.ts new file mode 100644 index 00000000000..f5c72354148 --- /dev/null +++ b/server/collaborator/src/__tests__/appendContent.test.ts @@ -0,0 +1,91 @@ +// +// Copyright © 2024 Hardcore Engineering Inc. +// +// Licensed under the Eclipse Public License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. You may +// obtain a copy of the License at https://www.eclipse.org/legal/epl-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import { XmlElement, XmlFragment, XmlText, Doc as YDoc, applyUpdate, encodeStateAsUpdate } from 'yjs' + +// These tests pin the one behavioural difference between updateContent and +// appendContent: both encode the incoming markup as a Y.js update and apply it +// inside the document's XmlFragment, but updateContent clears the fragment +// first (fragment.delete(0, fragment.length)) while appendContent does not. +// Clearing => replace; not clearing => the CRDT merges the new content in, +// giving append semantics while preserving concurrent edits. + +const field = 'content' + +function makeDoc (text: string): YDoc { + const doc = new YDoc() + const fragment = doc.getXmlFragment(field) + const paragraph = new XmlElement('paragraph') + paragraph.insert(0, [new XmlText(text)]) + fragment.insert(0, [paragraph]) + return doc +} + +function textOf (doc: YDoc): string { + return doc.getXmlFragment(field).toString() +} + +describe('appendContent semantics', () => { + it('append (no fragment.delete) into an empty document adds the new content', () => { + const target = new YDoc() + const incoming = makeDoc('new') + + applyUpdate(target, encodeStateAsUpdate(incoming)) + + expect(textOf(target)).toContain('new') + }) + + it('append (no fragment.delete) preserves prior content and adds the new content', () => { + const target = makeDoc('existing') + const incoming = makeDoc('new') + + applyUpdate(target, encodeStateAsUpdate(incoming)) + + const result = textOf(target) + expect(result).toContain('existing') + expect(result).toContain('new') + }) + + it('update (with fragment.delete) replaces prior content', () => { + const target = makeDoc('existing') + const incoming = makeDoc('new') + + target.transact(() => { + const fragment: XmlFragment = target.getXmlFragment(field) + fragment.delete(0, fragment.length) + applyUpdate(target, encodeStateAsUpdate(incoming)) + }) + + const result = textOf(target) + expect(result).not.toContain('existing') + expect(result).toContain('new') + }) + + it('two independent appends both land (CRDT merge, no clobber)', () => { + const target = makeDoc('base') + const appendA = makeDoc('alpha') + const appendB = makeDoc('beta') + + // Simulate two automations appending concurrently: each encodes against a + // fresh doc and its update is merged into the shared target. + applyUpdate(target, encodeStateAsUpdate(appendA)) + applyUpdate(target, encodeStateAsUpdate(appendB)) + + const result = textOf(target) + expect(result).toContain('base') + expect(result).toContain('alpha') + expect(result).toContain('beta') + }) +}) diff --git a/server/collaborator/src/rpc/methods/appendContent.ts b/server/collaborator/src/rpc/methods/appendContent.ts new file mode 100644 index 00000000000..e42933ff410 --- /dev/null +++ b/server/collaborator/src/rpc/methods/appendContent.ts @@ -0,0 +1,67 @@ +// +// Copyright © 2024 Hardcore Engineering Inc. +// +// Licensed under the Eclipse Public License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. You may +// obtain a copy of the License at https://www.eclipse.org/legal/epl-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import { type AppendContentRequest, type AppendContentResponse } from '@hcengineering/collaborator-client' +import { MeasureContext } from '@hcengineering/core' +import { applyUpdate, encodeStateAsUpdate } from 'yjs' +import { Context } from '../../context' +import { RpcMethodParams } from '../rpc' + +// Like updateContent, but WITHOUT clearing the existing fragment first. Applying +// the incoming update onto the untouched Y.XmlFragment merges (appends) the new +// content into whatever is already there, instead of replacing it. Because the +// mutation runs inside the same hocuspocus direct connection updateContent uses, +// it is race-free against concurrent browser editors via the Y.js CRDT layer. +export async function appendContent ( + ctx: MeasureContext, + context: Context, + documentName: string, + payload: AppendContentRequest, + params: RpcMethodParams +): Promise { + const { content } = payload + const { hocuspocus, transformer } = params + + const updates = ctx.withSync('transform', {}, () => { + const updates: Record = {} + + Object.entries(content).forEach(([field, markup]) => { + const ydoc = transformer.toYdoc(markup, field) + updates[field] = encodeStateAsUpdate(ydoc) + }) + + return updates + }) + + const connection = await ctx.with('connect', {}, () => { + return hocuspocus.openDirectConnection(documentName, context) + }) + + try { + await ctx.with('append', {}, () => + connection.transact((document) => { + document.transact(() => { + Object.entries(updates).forEach(([field, update]) => { + applyUpdate(document, update) + }) + }, connection) + }) + ) + } finally { + await connection.disconnect() + } + + return {} +} diff --git a/server/collaborator/src/rpc/methods/index.ts b/server/collaborator/src/rpc/methods/index.ts index 2a01f162807..067f8964adb 100644 --- a/server/collaborator/src/rpc/methods/index.ts +++ b/server/collaborator/src/rpc/methods/index.ts @@ -16,10 +16,12 @@ import { getContent } from './getContent' import { createContent } from './createContent' import { updateContent } from './updateContent' +import { appendContent } from './appendContent' import { RpcMethod } from '../rpc' export const methods: Record = { getContent, createContent, - updateContent + updateContent, + appendContent }