diff --git a/apps/server/src/collaboration/collaboration.gateway.ts b/apps/server/src/collaboration/collaboration.gateway.ts index f681686d..110d3a18 100644 --- a/apps/server/src/collaboration/collaboration.gateway.ts +++ b/apps/server/src/collaboration/collaboration.gateway.ts @@ -27,6 +27,7 @@ import { writeTitleFragment, } from './collaboration.handler'; import { User } from '@docmost/db/types/entity.types'; +import * as Y from 'yjs'; @Injectable() export class CollaborationGateway { @@ -184,7 +185,32 @@ export class CollaborationGateway { context ?? {}, ); try { - await connection.transact((doc) => writeTitleFragment(doc, title)); + // Write the new title into the in-memory 'title' fragment AND capture the + // resulting full doc state so we can persist it directly below. + let ydocState: Buffer | null = null; + await connection.transact((doc) => { + writeTitleFragment(doc, title); + ydocState = Buffer.from(Y.encodeStateAsUpdate(doc)); + }); + + // F1 (variant C): persist the 'title' fragment to `page.ydoc` DIRECTLY, + // bypassing onStoreDocument. PageService.update already wrote the new title + // to the page.title COLUMN before calling this, so onStoreDocument's no-op + // fast-path (titleText === column) would NOT persist the in-memory fragment + // on disconnect — leaving the stored ydoc with the OLD title, which a later + // body edit would then revert the column back to. Writing the ydoc here + // makes BOTH column and persisted fragment consistent (NEW = NEW). + // + // Safe with or without a live editor: the write is idempotent and carries + // no tree snapshot (no double broadcast); when an editor is connected, the + // normal onStoreDocument flow still persists the (superset) state later and + // the live clients receive the title change through the transact above. + if (ydocState) { + await this.persistenceExtension.persistTitleFragmentYdoc( + pageId, + ydocState, + ); + } } finally { await connection.disconnect(); } diff --git a/apps/server/src/collaboration/collaboration.handler.spec.ts b/apps/server/src/collaboration/collaboration.handler.spec.ts index 8d1fff15..27101f00 100644 --- a/apps/server/src/collaboration/collaboration.handler.spec.ts +++ b/apps/server/src/collaboration/collaboration.handler.spec.ts @@ -85,15 +85,26 @@ describe('CollaborationGateway.writePageTitle — Redis-independent path', () => // redisSync is intentionally null — this is the no-Redis scenario. gateway.redisSync = null; gateway.hocuspocus = { openDirectConnection } as any; + // F1 (variant C): writePageTitle persists the 'title' fragment directly so a + // later body edit can't revert the rename (see title-rename-durability.spec). + const persistTitleFragmentYdoc = jest.fn().mockResolvedValue(undefined); + gateway.persistenceExtension = { persistTitleFragmentYdoc } as any; - return { gateway, openDirectConnection, transact, disconnect }; + return { + gateway, + openDirectConnection, + transact, + disconnect, + persistTitleFragmentYdoc, + }; }; it('writes the new title via openDirectConnection and disconnects', async () => { const doc = new Y.Doc(); Y.applyUpdate(doc, Y.encodeStateAsUpdate(buildTitleSeedYdoc('Old Title'))); - const { gateway, openDirectConnection, disconnect } = makeGateway(doc); + const { gateway, openDirectConnection, disconnect, persistTitleFragmentYdoc } = + makeGateway(doc); await gateway.writePageTitle('page-1', 'New Title', { user: { id: 'u1' } }); @@ -102,6 +113,11 @@ describe('CollaborationGateway.writePageTitle — Redis-independent path', () => expect.objectContaining({ user: { id: 'u1' } }), ); expect(readTitleText(doc)).toBe('New Title'); + // The renamed fragment is persisted directly to page.ydoc (F1 variant C). + expect(persistTitleFragmentYdoc).toHaveBeenCalledWith( + 'page-1', + expect.any(Buffer), + ); expect(disconnect).toHaveBeenCalledTimes(1); }); diff --git a/apps/server/src/collaboration/extensions/persistence.extension.ts b/apps/server/src/collaboration/extensions/persistence.extension.ts index 4b16b2bf..45df5ecc 100644 --- a/apps/server/src/collaboration/extensions/persistence.extension.ts +++ b/apps/server/src/collaboration/extensions/persistence.extension.ts @@ -290,6 +290,35 @@ export class PersistenceExtension implements Extension { } } + /** + * Persist an already-encoded Y.Doc state directly to `page.ydoc`, mirroring the + * `pageRepo.updatePage({ ydoc })` write that onStoreDocument uses. + * + * Used by the gateway's writePageTitle (F1, variant C). A REST/MCP/agent rename + * with no live editor writes the new title into the in-memory 'title' fragment, + * but onStoreDocument's no-op fast-path (page.title column already equals the + * new title) does NOT persist that in-memory fragment, so the stored `page.ydoc` + * keeps the OLD title — and a later body edit then reverts the rename (loads the + * OLD fragment, sees it differs from the column, overwrites the column back to + * OLD). Writing the ydoc here keeps the persisted fragment consistent with the + * column so the rename survives. + * + * Broadcast-safe / no double broadcast: this carries no `treeUpdate`, so the + * tree WS + redis listeners (which gate on `treeUpdate`) do NOT re-broadcast the + * rename — only PageService.update's own PAGE_UPDATED does. The only extra + * side-effect is an idempotent search reindex. + * + * Idempotent and lock-free, so it is safe whether or not a live editor is + * connected: Yjs state is cumulative, so a concurrent onStoreDocument simply + * persists a superset of this state later. + */ + async persistTitleFragmentYdoc( + pageId: string, + ydocState: Buffer, + ): Promise { + await this.pageRepo.updatePage({ ydoc: ydocState }, pageId); + } + async onStoreDocument(data: onStoreDocumentPayload) { const { documentName, document, context } = data; diff --git a/apps/server/src/collaboration/title-rename-durability.spec.ts b/apps/server/src/collaboration/title-rename-durability.spec.ts new file mode 100644 index 00000000..f0d3b2aa --- /dev/null +++ b/apps/server/src/collaboration/title-rename-durability.spec.ts @@ -0,0 +1,187 @@ +import * as Y from 'yjs'; +import { TiptapTransformer } from '@hocuspocus/transformer'; +import { CollaborationGateway } from './collaboration.gateway'; +import { PersistenceExtension } from './extensions/persistence.extension'; +import { + buildTitleSeedYdoc, + jsonToText, + tiptapExtensions, +} from './collaboration.util'; + +/** + * F1 (variant C) — rename durability for a page with an already-persisted Yjs + * 'title' fragment and NO live editor (the REST/MCP/agent rename path). + * + * The bug: PageService.update writes the NEW title to the `page.title` COLUMN, + * then calls gateway.writePageTitle, which loads the page's ydoc (fragment = + * OLD) and overwrites it to NEW in memory. On disconnect, onStoreDocument sees + * titleText(NEW) === column(NEW) → no-op fast-path → it does NOT persist the + * in-memory fragment. So `page.ydoc` keeps the OLD title, and a LATER body edit + * loads the OLD fragment, sees it differs from the column, and silently reverts + * the column back to OLD. + * + * The fix: writePageTitle persists the 'title' fragment to `page.ydoc` DIRECTLY + * (via PersistenceExtension.persistTitleFragmentYdoc) after the transact, so the + * persisted fragment and the column stay consistent. + * + * This test drives the REAL writePageTitle + the REAL onStoreDocument against an + * in-memory page row, so it FAILS on the pre-fix no-op behaviour and PASSES after. + */ + +const PAGE_ID = '550e8400-e29b-41d4-a716-446655440000'; +const USER_ID = 'user-1'; +const OLD_TITLE = 'Old Title'; +const NEW_TITLE = 'Renamed Title'; + +const bodyJson = (text: string) => ({ + type: 'doc', + content: [{ type: 'paragraph', content: [{ type: 'text', text }] }], +}); + +// Build the initial persisted ydoc carrying BOTH a 'title' fragment and a body. +const makeInitialYdoc = (title: string, body: any): Buffer => { + const doc = new Y.Doc(); + Y.applyUpdate(doc, Y.encodeStateAsUpdate(buildTitleSeedYdoc(title))); + Y.applyUpdate( + doc, + Y.encodeStateAsUpdate(TiptapTransformer.toYdoc(body, 'default', tiptapExtensions)), + ); + return Buffer.from(Y.encodeStateAsUpdate(doc)); +}; + +// Load a doc from a persisted buffer (mirrors openDirectConnection loading from +// persistence when no editor is connected). hocuspocus augments the live doc +// with broadcastStateless(); a bare Y.Doc lacks it, so stub it. +const loadDoc = (buf: Buffer): Y.Doc => { + const doc = new Y.Doc(); + if (buf) Y.applyUpdate(doc, new Uint8Array(buf)); + (doc as any).broadcastStateless = jest.fn(); + return doc; +}; + +// Read the 'title' fragment text from a persisted buffer. +const readTitle = (buf: Buffer): string => { + const doc = loadDoc(buf); + const titleJson = TiptapTransformer.fromYdoc(doc, 'title'); + return titleJson ? jsonToText(titleJson).trim() : ''; +}; + +describe('rename durability (F1 variant C): persisted title fragment survives a body edit', () => { + it('persists the renamed title into page.ydoc so a later body edit does not revert it', async () => { + // In-memory page row = the DB. + const row: any = { + id: PAGE_ID, + slugId: 'slug-1', + spaceId: 'space-1', + workspaceId: 'ws-1', + creatorId: 'creator-1', + contributorIds: ['creator-1'], + createdAt: new Date('2020-01-01T00:00:00Z'), + lastUpdatedSource: 'user', + title: OLD_TITLE, + // content column mirrors the normalized body in the ydoc. + content: TiptapTransformer.fromYdoc( + loadDoc(makeInitialYdoc(OLD_TITLE, bodyJson('BODY V1'))), + 'default', + ), + ydoc: makeInitialYdoc(OLD_TITLE, bodyJson('BODY V1')), + }; + + const pageRepo = { + findById: jest.fn(async () => ({ ...row })), + updatePage: jest.fn(async (data: any, _pageId?: string) => { + Object.assign(row, data, { updatedAt: new Date() }); + }), + }; + const pageHistoryRepo = { + saveHistory: jest.fn().mockResolvedValue(undefined), + findPageLastHistory: jest.fn().mockResolvedValue(null), + }; + const noopQueue = { add: jest.fn().mockResolvedValue(undefined) }; + const collabHistory = { addContributors: jest.fn().mockResolvedValue(undefined) }; + const transclusionService = { + syncPageTransclusions: jest.fn().mockResolvedValue(undefined), + syncPageReferences: jest.fn().mockResolvedValue(undefined), + syncPageTemplateReferences: jest.fn().mockResolvedValue(undefined), + }; + // db whose transaction().execute(fn) runs fn with a trx stub (drives the + // real executeTx helper without a database). + const db = { + transaction: () => ({ + execute: (fn: (trx: any) => Promise) => fn({ __trx: true }), + }), + }; + + const ext = new PersistenceExtension( + pageRepo as any, + pageHistoryRepo as any, + db as any, + noopQueue as any, + noopQueue as any, + noopQueue as any, + collabHistory as any, + transclusionService as any, + ); + jest.spyOn(ext['logger'], 'debug').mockImplementation(() => undefined); + jest.spyOn(ext['logger'], 'warn').mockImplementation(() => undefined); + jest.spyOn(ext['logger'], 'error').mockImplementation(() => undefined); + + const documentName = `page.${PAGE_ID}`; + // Fake hocuspocus: openDirectConnection loads a doc from the CURRENT persisted + // ydoc (no live editor) and, on disconnect, runs the real onStoreDocument — + // exactly the no-live-editor unload path. + const fakeHocuspocus = { + openDirectConnection: jest.fn(async (name: string, context: any) => { + const liveDoc = loadDoc(row.ydoc); + return { + transact: async (fn: (doc: Y.Doc) => void) => fn(liveDoc), + disconnect: async () => { + await ext.onStoreDocument({ + documentName: name, + document: liveDoc, + context, + } as any); + }, + }; + }), + }; + + const gateway: CollaborationGateway = Object.create( + CollaborationGateway.prototype, + ); + (gateway as any).hocuspocus = fakeHocuspocus; + (gateway as any).persistenceExtension = ext; + + // --- REST/service rename (no live editor) --- + // 1) PageService.update writes the NEW title to the column. + await pageRepo.updatePage({ title: NEW_TITLE }, PAGE_ID); + // 2) PageService.update syncs the Yjs 'title' fragment. + await gateway.writePageTitle(PAGE_ID, NEW_TITLE, { + user: { id: USER_ID } as any, + }); + + // Reload the persisted ydoc: the 'title' fragment must now be NEW. + // (Pre-fix this is still OLD — writePageTitle did not persist the fragment.) + expect(readTitle(row.ydoc)).toBe(NEW_TITLE); + + // --- a later body edit must NOT revert the title --- + const editDoc = loadDoc(row.ydoc); + const frag = editDoc.getXmlFragment('default'); + const p = new Y.XmlElement('paragraph'); + const t = new Y.XmlText(); + t.insert(0, 'appended'); + p.insert(0, [t]); + frag.insert(frag.length, [p]); + + await ext.onStoreDocument({ + documentName, + document: editDoc, + context: { user: { id: USER_ID } }, + } as any); + + // The body edit was persisted, and the title stayed NEW in BOTH the column + // and the persisted ydoc fragment (pre-fix the column reverts to OLD). + expect(row.title).toBe(NEW_TITLE); + expect(readTitle(row.ydoc)).toBe(NEW_TITLE); + }); +});