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 <noreply@anthropic.com>
This commit is contained in:
claude code agent 227
2026-06-21 03:08:34 +03:00
parent 05a7a4001f
commit 212bcea4d7

View File

@@ -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'; const isAgent = provenance?.actor === 'agent';
await this.pageRepo.updatePage( const updateResult = await this.pageRepo.updatePage(
{ {
position: dto.position, position: dto.position,
parentPageId: parentPageId, parentPageId: parentPageId,
@@ -982,6 +1000,13 @@ export class PageService {
dto.pageId, 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 // The generic PAGE_UPDATED emitted by updatePage above is intentionally NOT
// used to drive the tree `moveTreeNode` broadcast: it also fires on rename / // used to drive the tree `moveTreeNode` broadcast: it also fires on rename /
// content-save and carries neither oldParentId nor the new position. Emit a // content-save and carries neither oldParentId nor the new position. Emit a