From 0eecdcba23a4a308360fde8b7fe8ecd0ec5cba97 Mon Sep 17 00:00:00 2001 From: claude code agent 227 Date: Tue, 30 Jun 2026 03:16:40 +0300 Subject: [PATCH] =?UTF-8?q?test(collab):=20cover=20the=20title-change=20?= =?UTF-8?q?=E2=8B=88=20empty-guard=20intersection=20(F1)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The develop rebase merged the #120 title-only-change branch and the #248/#251 store-side empty-guard into one onStoreDocument. The existing 14 tests exercise each only in isolation (empty-guard tests send no title fragment; title tests send a non-empty body), so none reached the empty-guard's blocking branch with titleChanged===true. Add two paired regression tests on that exact junction: - empty body + a changed non-empty title over non-empty persisted content, no intentional-clear → the empty-guard blocks the WHOLE store, dropping the simultaneous rename too (updatePage not called); the rich content and old title survive. - the same doc with a deliberate clear armed via the real stateless transport → the empty body is allowed and the rename rides along on the same body-path updatePage (title + empty content persisted). The pair makes Test 1 non-vacuous: same doc, only the clear differs, and Test 2 proves updatePage IS reachable — so Test 1's "not called" is the guard blocking, not an unreached path. Test-only; no production change. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../extensions/persistence-store.spec.ts | 78 ++++++++++++++++++- 1 file changed, 77 insertions(+), 1 deletion(-) diff --git a/apps/server/src/collaboration/extensions/persistence-store.spec.ts b/apps/server/src/collaboration/extensions/persistence-store.spec.ts index 1f1b5728..f4a7d0c7 100644 --- a/apps/server/src/collaboration/extensions/persistence-store.spec.ts +++ b/apps/server/src/collaboration/extensions/persistence-store.spec.ts @@ -1,6 +1,7 @@ +import * as Y from 'yjs'; import { TiptapTransformer } from '@hocuspocus/transformer'; import { PersistenceExtension } from './persistence.extension'; -import { tiptapExtensions } from '../collaboration.util'; +import { buildTitleSeedYdoc, tiptapExtensions } from '../collaboration.util'; /** * Integration test for `onStoreDocument`'s Approach-A boundary snapshot. @@ -403,6 +404,81 @@ describe('PersistenceExtension.onStoreDocument — Approach-A boundary snapshot' expect(pageRepo.updatePage).not.toHaveBeenCalled(); }); + // #248/#251 ⋈ title-change INTERSECTION — the empty-guard must dominate a + // simultaneous rename. The two guards merged by the rebase are exercised in + // isolation everywhere else: every empty-guard test sends a body with NO title + // fragment, and every title test sends a NON-empty body. This test hits their + // intersection: an empty body ('default') carrying a CHANGED, non-empty title + // fragment ('New Title' over an OLD 'Old Title') landing on non-empty persisted + // content, with NO intentional-clear. Because the body is empty, + // bodyChanged===true, so the store goes down the BODY path (not the title-only + // branch, which requires !bodyChanged) and reaches the empty-guard while + // titleChanged===true. The guard must block the WHOLE store: the rename rides on + // the same updatePage as the body, so persisting the title would also wipe the + // body. Nothing may be written — the rich content (and old title) survive. + it('blocks the whole store — empty body drops a simultaneous rename too (#248/#251 ⋈ title)', async () => { + // Empty body in the 'default' fragment... + const document = ydocFor({ type: 'doc', content: [{ type: 'paragraph' }] }); + // ...plus a CHANGED, non-empty title fragment in the SAME Y.Doc. + Y.applyUpdate(document, Y.encodeStateAsUpdate(buildTitleSeedYdoc('New Title'))); + (document as any).broadcastStateless = + (document as any).broadcastStateless || jest.fn(); + + // Persisted content is non-empty AND titled differently, so both + // bodyChanged and titleChanged are true when the store runs. + pageRepo.findById.mockResolvedValue({ + ...persistedHumanPage('IGNORED'), + content: doc('IMPORTANT RICH CONTENT'), + title: 'Old Title', + }); + + // No intentional-clear signalled → the empty-guard blocks. + await ext.onStoreDocument(buildData(document, 'user') as any); + + // Neither the body nor the new title is written; the rich content and the + // old title both survive. (Non-vacuity: if the guard let the rename through, + // updatePage would be called with title:'New Title' on the body path.) + expect(pageRepo.updatePage).not.toHaveBeenCalled(); + }); + + // #248/#251 ⋈ title INTERSECTION, allowed side — a deliberate clear lets BOTH + // the empty body AND the simultaneous rename through. Same doc as above (empty + // body + changed 'New Title'), but the clear is armed first via the REAL + // stateless transport seam (exactly as the #251 tests do). The body path then + // persists once, and because titleChanged===true the same updatePage carries + // the new title — the rename rides along with the deliberate clear. + it('allows the empty body AND the rename when an intentional clear is signalled (#248/#251 ⋈ title)', async () => { + const documentName = `page.${PAGE_ID}`; + const document = ydocFor({ type: 'doc', content: [{ type: 'paragraph' }] }); + Y.applyUpdate(document, Y.encodeStateAsUpdate(buildTitleSeedYdoc('New Title'))); + (document as any).broadcastStateless = + (document as any).broadcastStateless || jest.fn(); + + pageRepo.findById.mockResolvedValue({ + ...persistedHumanPage('IGNORED'), + content: doc('IMPORTANT RICH CONTENT'), + title: 'Old Title', + }); + + // The client signalled a deliberate clear over the live connection. + await ext.onStateless({ + connection: { readOnly: false } as any, + documentName, + document: document as any, + payload: JSON.stringify({ type: 'intentional-clear' }), + } as any); + + await ext.onStoreDocument(buildData(document, 'user') as any); + + // The empty body was allowed and the rename rode along on the same write. + expect(pageRepo.updatePage).toHaveBeenCalledTimes(1); + expect(pageRepo.updatePage.mock.calls[0][0].title).toBe('New Title'); + // Pin that this went down the BODY path (not the title-only branch): the + // persisted content is the empty doc, i.e. the deliberate clear was written. + const expectedEmpty = TiptapTransformer.fromYdoc(document, 'default'); + expect(pageRepo.updatePage.mock.calls[0][0].content).toEqual(expectedEmpty); + }); + // persist-1 — when every attempt fails the hook must NOT report a phantom // success: no "page.updated" badge broadcast and no history snapshot for // content that was never written.