From 212bcea4d7d470d6a41169609224df9bd45068fa Mon Sep 17 00:00:00 2001 From: claude code agent 227 Date: Sun, 21 Jun 2026 03:08:34 +0300 Subject: [PATCH] fix(page): movePage cycle guard + no phantom PAGE_MOVED (#67, #64) #67: movePage didn't check the destination wasn't the page itself or inside its own subtree, so MCP/REST/agent/fast-drag could persist+broadcast a cycle. Reject before the update (self-parent, or moved page among the destination parent's ancestors via getPageBreadCrumbs). #64: movePage emitted PAGE_MOVED from a stale pre-read even when the row didn't change / was concurrently deleted (phantom move). Gate the emit on updateResult.numUpdatedRows !== 0n. Both are movePage hardening in one method. Co-Authored-By: Claude Opus 4.8 --- .../src/core/page/services/page.service.ts | 27 ++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/apps/server/src/core/page/services/page.service.ts b/apps/server/src/core/page/services/page.service.ts index dc17e188..bc6e5105 100644 --- a/apps/server/src/core/page/services/page.service.ts +++ b/apps/server/src/core/page/services/page.service.ts @@ -964,9 +964,27 @@ export class PageService { } } + // Server-side cycle guard: a page may not be moved into itself or into any + // page within its own subtree. Without this, an MCP/REST/agent caller (or a + // fast drag racing the client check) could persist a cycle and broadcast it. + // Only relevant when re-parenting under a concrete parent; moving to root + // (parentPageId null/undefined) can never create a cycle. + if (dto.parentPageId) { + if (dto.parentPageId === dto.pageId) { + throw new BadRequestException('Cannot move a page into its own subtree'); + } + // Walk the destination parent's ancestor chain (reusing the breadcrumb + // ancestor CTE). If the page being moved appears among those ancestors, + // the destination lives inside the moved page's subtree -> cycle. + const destAncestors = await this.getPageBreadCrumbs(dto.parentPageId); + if (destAncestors.some((ancestor) => ancestor.id === dto.pageId)) { + throw new BadRequestException('Cannot move a page into its own subtree'); + } + } + const isAgent = provenance?.actor === 'agent'; - await this.pageRepo.updatePage( + const updateResult = await this.pageRepo.updatePage( { position: dto.position, parentPageId: parentPageId, @@ -982,6 +1000,13 @@ export class PageService { dto.pageId, ); + // Guard against a phantom broadcast: if the row was concurrently deleted or + // otherwise not updated, skip the PAGE_MOVED event so we don't replay a move + // built from the stale pre-read snapshot to every connected client. + if (!updateResult || updateResult.numUpdatedRows === 0n) { + return; + } + // The generic PAGE_UPDATED emitted by updatePage above is intentionally NOT // used to drive the tree `moveTreeNode` broadcast: it also fires on rename / // content-save and carries neither oldParentId nor the new position. Emit a