From 98769155d3bf5efdeafdcf54286ecb92953513e9 Mon Sep 17 00:00:00 2001 From: claude code agent 227 Date: Sat, 20 Jun 2026 22:37:35 +0300 Subject: [PATCH] test(page-templates): cover client pageEmbed cycle/self-embed/depth guard (#31) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The cycle/self-embed/depth guard (PAGE_EMBED_MAX_DEPTH=5) lives only on the client and is the sole protection against runaway nested rendering — and was untested. Extract the inline predicates into pure, behavior-identical exported helpers (isPageEmbedCycle, isPageEmbedTooDeep in the ancestry context; filterPageEmbedOptions in the picker) so they're unit-testable without mounting the heavy Tiptap NodeView, and add vitest coverage (20 tests): ancestry chain/ host accumulation, cycle (ancestor-in-chain + top-level self-embed), too-deep at the cap, and picker host-exclusion. Co-Authored-By: Claude Opus 4.8 --- .../page-embed-ancestry-context.test.tsx | 149 ++++++++++++++++++ .../page-embed-ancestry-context.tsx | 23 +++ .../page-embed/page-embed-picker.test.ts | 44 ++++++ .../page-embed/page-embed-picker.tsx | 16 +- .../components/page-embed/page-embed-view.tsx | 14 +- 5 files changed, 237 insertions(+), 9 deletions(-) create mode 100644 apps/client/src/features/editor/components/page-embed/page-embed-ancestry-context.test.tsx create mode 100644 apps/client/src/features/editor/components/page-embed/page-embed-picker.test.ts diff --git a/apps/client/src/features/editor/components/page-embed/page-embed-ancestry-context.test.tsx b/apps/client/src/features/editor/components/page-embed/page-embed-ancestry-context.test.tsx new file mode 100644 index 00000000..867922d2 --- /dev/null +++ b/apps/client/src/features/editor/components/page-embed/page-embed-ancestry-context.test.tsx @@ -0,0 +1,149 @@ +import { describe, it, expect } from "vitest"; +import { render, screen } from "@testing-library/react"; +import { + PageEmbedAncestryProvider, + usePageEmbedAncestry, + isPageEmbedCycle, + isPageEmbedTooDeep, + PAGE_EMBED_MAX_DEPTH, +} from "./page-embed-ancestry-context"; + +/** + * Tiny probe that renders the current ancestry context as serialized data + * attributes so tests can assert the accumulated chain / threaded hostPageId + * without mounting the heavy Tiptap node view. + */ +function AncestryProbe({ testId = "probe" }: { testId?: string }) { + const { chain, hostPageId } = usePageEmbedAncestry(); + return ( + + ); +} + +describe("PageEmbedAncestryProvider", () => { + it("defaults to an empty chain and null host with no provider", () => { + render(); + const probe = screen.getByTestId("probe"); + expect(probe.getAttribute("data-chain")).toBe(""); + expect(probe.getAttribute("data-chain-length")).toBe("0"); + expect(probe.getAttribute("data-host")).toBe(""); + }); + + it("accumulates sourcePageId into the chain across nested providers", () => { + render( + + + + + + + , + ); + const probe = screen.getByTestId("probe"); + // Chain is built outermost -> innermost. + expect(probe.getAttribute("data-chain")).toBe("a,b,c"); + expect(probe.getAttribute("data-chain-length")).toBe("3"); + }); + + it("threads the host page id from the outermost provider down the tree", () => { + render( + + + + + , + ); + const probe = screen.getByTestId("probe"); + // The first host wins (parent.hostPageId ?? hostPageId); deeper hosts are + // ignored so the original host is preserved for self-embed detection. + expect(probe.getAttribute("data-host")).toBe("host-page"); + }); + + it("does not add an entry to the chain when sourcePageId is missing", () => { + render( + + + + + + + , + ); + const probe = screen.getByTestId("probe"); + // null / undefined sources are pass-through: chain stays ["a"], host kept. + expect(probe.getAttribute("data-chain")).toBe("a"); + expect(probe.getAttribute("data-host")).toBe("host"); + }); + + it("adopts a host provided only at a deeper level when the root had none", () => { + render( + + + + + , + ); + const probe = screen.getByTestId("probe"); + expect(probe.getAttribute("data-host")).toBe("late-host"); + }); +}); + +describe("isPageEmbedCycle", () => { + it("is false when the source is not in the chain and is not the host", () => { + expect(isPageEmbedCycle(["a", "b"], "host", "c")).toBe(false); + }); + + it("is true when the source is already present in the ancestor chain", () => { + expect(isPageEmbedCycle(["a", "b", "c"], "host", "b")).toBe(true); + }); + + it("is true for a top-level self-embed (host === source, empty chain)", () => { + expect(isPageEmbedCycle([], "self", "self")).toBe(true); + }); + + it("is true when the source equals the host even mid-chain", () => { + expect(isPageEmbedCycle(["x"], "self", "self")).toBe(true); + }); + + it("is false when there is no source id (nothing to embed yet)", () => { + expect(isPageEmbedCycle(["a"], "host", null)).toBe(false); + expect(isPageEmbedCycle([], "host", "")).toBe(false); + }); + + it("is false when host is null and source is not in the chain", () => { + expect(isPageEmbedCycle(["a", "b"], null, "c")).toBe(false); + }); +}); + +describe("isPageEmbedTooDeep", () => { + it("is false below the max depth", () => { + expect(isPageEmbedTooDeep([])).toBe(false); + expect( + isPageEmbedTooDeep(new Array(PAGE_EMBED_MAX_DEPTH - 1).fill("x")), + ).toBe(false); + }); + + it("is true once the chain length reaches the max depth", () => { + expect( + isPageEmbedTooDeep(new Array(PAGE_EMBED_MAX_DEPTH).fill("x")), + ).toBe(true); + }); + + it("is true when the chain length exceeds the max depth", () => { + expect( + isPageEmbedTooDeep(new Array(PAGE_EMBED_MAX_DEPTH + 3).fill("x")), + ).toBe(true); + }); + + it("guards at exactly PAGE_EMBED_MAX_DEPTH (=5)", () => { + // Pin the documented constant so an accidental change is caught. + expect(PAGE_EMBED_MAX_DEPTH).toBe(5); + expect(isPageEmbedTooDeep(["1", "2", "3", "4"])).toBe(false); + expect(isPageEmbedTooDeep(["1", "2", "3", "4", "5"])).toBe(true); + }); +}); diff --git a/apps/client/src/features/editor/components/page-embed/page-embed-ancestry-context.tsx b/apps/client/src/features/editor/components/page-embed/page-embed-ancestry-context.tsx index c989ee21..cdd7f109 100644 --- a/apps/client/src/features/editor/components/page-embed/page-embed-ancestry-context.tsx +++ b/apps/client/src/features/editor/components/page-embed/page-embed-ancestry-context.tsx @@ -51,3 +51,26 @@ export function PageEmbedAncestryProvider({ export function usePageEmbedAncestry() { return useContext(PageEmbedAncestryContext); } + +/** + * Pure cycle predicate used by the page-embed node view. Returns true when the + * source page would recurse into itself: either it is already present in the + * ancestor chain, or it is the host page (top-level self-embed). Extracted so + * the anti-DoS guard can be unit-tested without mounting the Tiptap NodeView. + */ +export function isPageEmbedCycle( + chain: string[], + hostPageId: string | null, + sourcePageId: string | null, +): boolean { + if (!sourcePageId) return false; + return chain.includes(sourcePageId) || hostPageId === sourcePageId; +} + +/** + * Pure depth-limit predicate. Returns true once the ancestor chain has reached + * the hard cap, before a deeper nested editor is mounted. + */ +export function isPageEmbedTooDeep(chain: string[]): boolean { + return chain.length >= PAGE_EMBED_MAX_DEPTH; +} diff --git a/apps/client/src/features/editor/components/page-embed/page-embed-picker.test.ts b/apps/client/src/features/editor/components/page-embed/page-embed-picker.test.ts new file mode 100644 index 00000000..257f09fd --- /dev/null +++ b/apps/client/src/features/editor/components/page-embed/page-embed-picker.test.ts @@ -0,0 +1,44 @@ +import { describe, it, expect } from "vitest"; +import { filterPageEmbedOptions } from "./page-embed-picker"; + +type Page = { id: string; title?: string }; + +describe("filterPageEmbedOptions", () => { + const pages: Page[] = [ + { id: "p1", title: "One" }, + { id: "host", title: "Host" }, + { id: "p2", title: "Two" }, + ]; + + it("excludes the host page from the options (self-embed guard)", () => { + const result = filterPageEmbedOptions(pages, "host"); + expect(result.map((p) => p.id)).toEqual(["p1", "p2"]); + }); + + it("keeps all pages when the host id matches nothing", () => { + const result = filterPageEmbedOptions(pages, "other"); + expect(result.map((p) => p.id)).toEqual(["p1", "host", "p2"]); + }); + + it("keeps all pages when no host id is provided", () => { + const result = filterPageEmbedOptions(pages, undefined); + expect(result.map((p) => p.id)).toEqual(["p1", "host", "p2"]); + }); + + it("drops nullish entries defensively", () => { + const dirty = [ + { id: "p1" }, + null as unknown as Page, + undefined as unknown as Page, + { id: "p2" }, + ]; + const result = filterPageEmbedOptions(dirty, "host"); + expect(result.map((p) => p.id)).toEqual(["p1", "p2"]); + }); + + it("returns an empty array for nullish input", () => { + expect( + filterPageEmbedOptions(null as unknown as Page[], "host"), + ).toEqual([]); + }); +}); diff --git a/apps/client/src/features/editor/components/page-embed/page-embed-picker.tsx b/apps/client/src/features/editor/components/page-embed/page-embed-picker.tsx index 7648b05e..5e914a57 100644 --- a/apps/client/src/features/editor/components/page-embed/page-embed-picker.tsx +++ b/apps/client/src/features/editor/components/page-embed/page-embed-picker.tsx @@ -9,6 +9,18 @@ import type { IPage } from "@/features/page/types/page.types"; export const PAGE_EMBED_PICKER_EVENT = "open-page-embed-picker"; +/** + * Pure filter excluding the host page (and any nullish entries) from the picker + * results. Extracted so the self-embed guard at insertion time is unit-testable + * without mounting the modal/search query. + */ +export function filterPageEmbedOptions( + pages: T[], + hostPageId?: string, +): T[] { + return (pages ?? []).filter((p) => p && p.id !== hostPageId); +} + type PickerDetail = { editor: Editor; range: Range; @@ -55,9 +67,7 @@ export default function PageEmbedPicker() { }); const hostPageId = detailRef.current?.hostPageId; - const pages = ((data?.pages ?? []) as IPage[]).filter( - (p) => p && p.id !== hostPageId, - ); + const pages = filterPageEmbedOptions((data?.pages ?? []) as IPage[], hostPageId); const handleSelect = (page: IPage) => { const detail = detailRef.current; diff --git a/apps/client/src/features/editor/components/page-embed/page-embed-view.tsx b/apps/client/src/features/editor/components/page-embed/page-embed-view.tsx index b51607db..a06a3063 100644 --- a/apps/client/src/features/editor/components/page-embed/page-embed-view.tsx +++ b/apps/client/src/features/editor/components/page-embed/page-embed-view.tsx @@ -20,7 +20,8 @@ import { usePageEmbedLookup } from "./page-embed-lookup-context"; import { PageEmbedAncestryProvider, usePageEmbedAncestry, - PAGE_EMBED_MAX_DEPTH, + isPageEmbedCycle, + isPageEmbedTooDeep, } from "./page-embed-ancestry-context"; import PageEmbedContent from "./page-embed-content"; @@ -100,11 +101,12 @@ function PageEmbedBody({ // --- Cycle / depth guard (evaluated before any lookup is rendered) --------- // Self-embed or a source already present in the ancestor chain → cycle. - const isCycle = - !!sourcePageId && - (ancestry.chain.includes(sourcePageId) || - ancestry.hostPageId === sourcePageId); - const isTooDeep = ancestry.chain.length >= PAGE_EMBED_MAX_DEPTH; + const isCycle = isPageEmbedCycle( + ancestry.chain, + ancestry.hostPageId, + sourcePageId, + ); + const isTooDeep = isPageEmbedTooDeep(ancestry.chain); const sourceTitle = result && !("status" in result) ? result.title : null;