From 6928817ceeb20deb49b8bf14d989714eecef445d Mon Sep 17 00:00:00 2001 From: claude code agent 227 Date: Sun, 21 Jun 2026 03:29:52 +0300 Subject: [PATCH] fix(ws): broadcast realtime page rename/icon change (#72) handleMessage became a no-op and PageWsListener intentionally ignored PAGE_UPDATED, so a rename/icon change (client operation:updateOne) was no longer rebroadcast -> other clients saw stale title/icon in the sidebar+breadcrumbs until a reload (create/duplicate/restore were covered; updateOne regressed). Add a server-authoritative onPageUpdated handler: PageService.update detects a real title/icon change (DTO carries the field AND value differs; no-op/content- only saves excluded) and attaches a treeUpdate snapshot to PAGE_UPDATED; the listener broadcasts a tree updateOne via the restriction-aware emitTreeEvent (so a restricted page's title never leaks). Content-only saves attach nothing. Co-Authored-By: Claude Opus 4.8 --- .../src/core/page/services/page.service.ts | 27 ++++++++++ .../src/database/listeners/page.listener.ts | 24 +++++++++ .../src/database/repos/page/page.repo.ts | 19 ++++++- .../src/ws/listeners/page-ws.listener.spec.ts | 51 +++++++++++++++++++ .../src/ws/listeners/page-ws.listener.ts | 21 ++++++-- apps/server/src/ws/ws-tree.service.ts | 24 +++++++++ 6 files changed, 161 insertions(+), 5 deletions(-) diff --git a/apps/server/src/core/page/services/page.service.ts b/apps/server/src/core/page/services/page.service.ts index b6de92a1..93a13bfe 100644 --- a/apps/server/src/core/page/services/page.service.ts +++ b/apps/server/src/core/page/services/page.service.ts @@ -270,6 +270,17 @@ export class PageService { const isAgent = provenance?.actor === 'agent'; + // Detect a real title/icon change so the WS tree listener can broadcast an + // `updateOne` to the space (rename / icon swap) WITHOUT re-broadcasting on a + // content-only save. Only treat a field as changed when the DTO actually + // carries it AND its value differs from what is already stored — a no-op + // save (same title, or a content-only update where these are undefined) + // produces no tree snapshot, so the listener stays quiet. + const titleChanged = + updatePageDto.title !== undefined && updatePageDto.title !== page.title; + const iconChanged = + updatePageDto.icon !== undefined && updatePageDto.icon !== page.icon; + await this.pageRepo.updatePage( { title: updatePageDto.title, @@ -287,6 +298,22 @@ export class PageService { contributorIds: contributorIds, }, page.id, + undefined, + // Enrich PAGE_UPDATED only when title/icon actually changed. The snapshot + // values come from the server-side data being persisted (DTO when present, + // otherwise the unchanged stored value), never relayed from the client. + titleChanged || iconChanged + ? { + treeUpdate: { + id: page.id, + slugId: page.slugId, + spaceId: page.spaceId, + parentPageId: page.parentPageId ?? null, + ...(titleChanged ? { title: updatePageDto.title } : {}), + ...(iconChanged ? { icon: updatePageDto.icon } : {}), + }, + } + : undefined, ); this.generalQueue diff --git a/apps/server/src/database/listeners/page.listener.ts b/apps/server/src/database/listeners/page.listener.ts index 2b67b364..3a779aa3 100644 --- a/apps/server/src/database/listeners/page.listener.ts +++ b/apps/server/src/database/listeners/page.listener.ts @@ -34,6 +34,30 @@ export class PageEvent { // Set on PAGE_RESTORED so the WS listener can scope a refetchRootTreeNodeEvent // to the affected space (restore can re-attach a whole subtree). spaceId?: string; + // Set on a PAGE_UPDATED that actually changed the page's title and/or icon + // (a rename or icon swap). Content-only saves leave this undefined, which is + // how the WS listener distinguishes a tree-relevant metadata change from a + // noisy content save and avoids re-broadcasting on every keystroke-flush. + // Server-authoritative: built from the values being persisted, not relayed + // from the client. + treeUpdate?: TreeUpdateSnapshot; +} + +/** + * Thin snapshot carried on a PAGE_UPDATED event when the title and/or icon + * changed, so the WS tree listener can broadcast an `updateOne` without a DB + * read. Only the fields the client tree receiver (`applyUpdateOne`) consumes + * are included. + */ +export interface TreeUpdateSnapshot { + id: string; + slugId: string; + spaceId: string; + parentPageId: string | null; + // Present only when that field actually changed; an undefined field is left + // untouched by the client reducer. + title?: string | null; + icon?: string | null; } /** diff --git a/apps/server/src/database/repos/page/page.repo.ts b/apps/server/src/database/repos/page/page.repo.ts index 504d01b3..51c6132b 100644 --- a/apps/server/src/database/repos/page/page.repo.ts +++ b/apps/server/src/database/repos/page/page.repo.ts @@ -16,6 +16,16 @@ import { jsonArrayFrom, jsonObjectFrom } from 'kysely/helpers/postgres'; import { SpaceMemberRepo } from '@docmost/db/repos/space/space-member.repo'; import { EventEmitter2 } from '@nestjs/event-emitter'; import { EventName } from '../../../common/events/event.contants'; +import { TreeUpdateSnapshot } from '../../listeners/page.listener'; + +/** + * Optional extras for the PAGE_UPDATED event emitted by updatePage(s). Lets the + * caller attach a tree snapshot for a title/icon change so the WS listener can + * broadcast an `updateOne` without re-reading the DB. + */ +export interface UpdatePageEventOpts { + treeUpdate?: TreeUpdateSnapshot; +} @Injectable() export class PageRepo { @@ -138,14 +148,16 @@ export class PageRepo { updatablePage: UpdatablePage, pageId: string, trx?: KyselyTransaction, + opts?: UpdatePageEventOpts, ) { - return this.updatePages(updatablePage, [pageId], trx); + return this.updatePages(updatablePage, [pageId], trx, opts); } async updatePages( updatePageData: UpdatablePage, pageIds: string[], trx?: KyselyTransaction, + opts?: UpdatePageEventOpts, ) { const result = await dbOrTx(this.db, trx) .updateTable('pages') @@ -160,6 +172,11 @@ export class PageRepo { this.eventEmitter.emit(EventName.PAGE_UPDATED, { pageIds: pageIds, workspaceId: updatePageData.workspaceId, + // Optional tree snapshot for the WS listener (variant A). The caller sets + // it ONLY for a title/icon change so the listener can broadcast an + // `updateOne` without a DB read; content-only saves omit it and the + // listener skips them. Built from server-side data, never client-relayed. + ...(opts?.treeUpdate ? { treeUpdate: opts.treeUpdate } : {}), }); return result; diff --git a/apps/server/src/ws/listeners/page-ws.listener.spec.ts b/apps/server/src/ws/listeners/page-ws.listener.spec.ts index 3282d318..cb8d8d90 100644 --- a/apps/server/src/ws/listeners/page-ws.listener.spec.ts +++ b/apps/server/src/ws/listeners/page-ws.listener.spec.ts @@ -5,6 +5,7 @@ import { PageEvent, PageMovedEvent, TreeNodeSnapshot, + TreeUpdateSnapshot, } from '../../database/listeners/page.listener'; const snapshot: TreeNodeSnapshot = { @@ -230,3 +231,53 @@ describe('PageWsListener delete/move/restore handlers', () => { expect(wsTree.broadcastRefetchRoot).toHaveBeenCalledWith('space-9'); }); }); + +describe('PageWsListener.onPageUpdated (rename / icon change)', () => { + let listener: PageWsListener; + let wsTree: { broadcastPageUpdated: jest.Mock }; + + const treeUpdate: TreeUpdateSnapshot = { + id: 'page-1', + slugId: 'slug-1', + spaceId: 'space-1', + parentPageId: null, + title: 'Renamed', + icon: '🚀', + }; + + beforeEach(async () => { + wsTree = { + broadcastPageUpdated: jest.fn().mockResolvedValue(undefined), + }; + + const module: TestingModule = await Test.createTestingModule({ + providers: [PageWsListener, { provide: WsTreeService, useValue: wsTree }], + }).compile(); + + listener = module.get(PageWsListener); + }); + + it('WITH a title/icon `treeUpdate`: broadcasts updateOne with that snapshot', async () => { + const event: PageEvent = { + pageIds: ['page-1'], + workspaceId: 'ws-1', + treeUpdate, + }; + + await listener.onPageUpdated(event); + + expect(wsTree.broadcastPageUpdated).toHaveBeenCalledTimes(1); + expect(wsTree.broadcastPageUpdated).toHaveBeenCalledWith(treeUpdate); + }); + + it('content-only save (NO `treeUpdate`): does NOT broadcast', async () => { + const event: PageEvent = { + pageIds: ['page-1'], + workspaceId: 'ws-1', + }; + + await listener.onPageUpdated(event); + + expect(wsTree.broadcastPageUpdated).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/server/src/ws/listeners/page-ws.listener.ts b/apps/server/src/ws/listeners/page-ws.listener.ts index 100d0f85..3de6da35 100644 --- a/apps/server/src/ws/listeners/page-ws.listener.ts +++ b/apps/server/src/ws/listeners/page-ws.listener.ts @@ -16,12 +16,14 @@ import { WsTreeService } from '../ws-tree.service'; * keeps it safe against the in-transaction visibility race (a synchronous * SELECT here could run before the emitting `trx` committed). * - * Scope of this PR: create, move, soft-delete/delete, restore. + * Scope: create, move, soft-delete/delete, restore, rename / icon change. + * + * Rename / icon change rides PAGE_UPDATED, which ALSO fires on every content + * save. The emit site (PageService.update) attaches a `treeUpdate` snapshot ONLY + * when the title or icon actually changed, so the handler below can gate strictly + * on that snapshot and stay silent on content-only saves. * * Deferred follow-ups (intentionally NOT handled here): - * - rename / icon change: would broadcast `updateOne` on PAGE_UPDATED, but - * PAGE_UPDATED also fires on every content save, so it needs a title/icon - * diff filter to avoid noise. * - cross-space move (`movePageToSpace` / PAGE_MOVED_TO_SPACE): needs a * deleteTreeNode in the old space + addTreeNode/refetch in the new space. */ @@ -68,6 +70,17 @@ export class PageWsListener { await this.wsTree.broadcastPageMoved(event); } + // Rename / icon change. PAGE_UPDATED also fires on every content save, so we + // only act when the emit site flagged a real title/icon change via + // `treeUpdate` — content-only saves carry no snapshot and are ignored here + // (no noisy re-broadcast). The broadcast is restriction-aware (emitTreeEvent), + // so a restricted page's title/icon can't leak to unauthorized sockets. + @OnEvent(EventName.PAGE_UPDATED) + async onPageUpdated(event: PageEvent): Promise { + if (!event.treeUpdate) return; + await this.wsTree.broadcastPageUpdated(event.treeUpdate); + } + @OnEvent(EventName.PAGE_RESTORED) async onPageRestored(event: PageEvent): Promise { // Restore can re-attach a whole subtree; a root refetch is simpler and more diff --git a/apps/server/src/ws/ws-tree.service.ts b/apps/server/src/ws/ws-tree.service.ts index 5fa8ef5d..5e052485 100644 --- a/apps/server/src/ws/ws-tree.service.ts +++ b/apps/server/src/ws/ws-tree.service.ts @@ -4,6 +4,7 @@ import { WsService } from './ws.service'; import { PageMovedEvent, TreeNodeSnapshot, + TreeUpdateSnapshot, } from '../database/listeners/page.listener'; @Injectable() @@ -43,6 +44,29 @@ export class WsTreeService { }); } + // Rename / icon change: patch the in-tree node's title/icon on every client in + // the space. Routed through the restriction-aware `emitTreeEvent` so a + // restricted page's new title/icon never leaks to sockets that can't see it. + // The payload mirrors the client `UpdateEvent` shape consumed by + // `applyUpdateOne` (entity ["pages"], `id`, `payload.title` / `payload.icon`); + // only the fields that actually changed are sent (the snapshot omits the rest). + async broadcastPageUpdated(node: TreeUpdateSnapshot): Promise { + await this.wsService.emitTreeEvent(node.spaceId, node.id, { + operation: 'updateOne', + spaceId: node.spaceId, + entity: ['pages'], + id: node.id, + payload: { + slugId: node.slugId, + parentPageId: node.parentPageId, + // Only include changed fields; an absent field leaves the client node + // untouched (applyUpdateOne checks `!== undefined` per field). + ...(node.title !== undefined ? { title: node.title } : {}), + ...(node.icon !== undefined ? { icon: node.icon } : {}), + }, + }); + } + async broadcastPageDeleted(page: TreeNodeSnapshot): Promise { await this.wsService.emitTreeEvent(page.spaceId, page.id, { operation: 'deleteTreeNode',