From e8805b39c8b5d40451f504dbc37c6fb2b3b94c65 Mon Sep 17 00:00:00 2001 From: claude code agent 227 Date: Mon, 29 Jun 2026 00:40:34 +0300 Subject: [PATCH] fix(collab): persist renamed title fragment to page.ydoc (F1, variant C) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A REST/MCP/agent rename (no live editor) wrote the new title to the page.title column, then writePageTitle loaded the ydoc (fragment still OLD) and set it to NEW only in memory. On disconnect onStoreDocument saw titleText(NEW) === column(NEW), took the no-op fast-path, and never persisted the in-memory fragment — so page.ydoc kept the OLD title and a later body edit silently reverted the column back to OLD. writePageTitle now persists the 'title' fragment to page.ydoc DIRECTLY (PersistenceExtension.persistTitleFragmentYdoc) after the transact, bypassing the no-op onStoreDocument. The write carries no treeUpdate, so the tree WS/redis listeners do not re-broadcast (no double broadcast), and it is idempotent/lock-free so it is safe whether or not a live editor is connected. Adds a persist-then-reload-then-edit-body regression test that fails on the old no-op behaviour and passes after the fix. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../collaboration/collaboration.gateway.ts | 28 ++- .../collaboration.handler.spec.ts | 20 +- .../extensions/persistence.extension.ts | 29 +++ .../title-rename-durability.spec.ts | 187 ++++++++++++++++++ 4 files changed, 261 insertions(+), 3 deletions(-) create mode 100644 apps/server/src/collaboration/title-rename-durability.spec.ts 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); + }); +});